[FFmpeg-devel] [PATCH 1/2] dshow: fix return code when opening device

[Ramiro Polla]

On Tue, Dec 4, 2012 at 9:58 PM, Don Moir <donmoir at comcast.net> wrote:
>> On date Tuesday 2012-12-04 04:22:06 -0200, Ramiro Polla encoded:
>>>
>>> Hi,
>>>
>>> I've sent this patch before, but I had forgotten why I had written it.
>>> Thanks to Don Moir for pointing out why it was necessary.
>>>
>>> Ramiro
>>
>>
>>> From d2d9631a110bbb28d5aa2082fe508df5739543ac Mon Sep 17 00:00:00 2001
>>> From: Ramiro Polla <ramiro.polla at gmail.com>
>>> Date: Tue, 4 Dec 2012 02:29:43 -0200
>>> Subject: [PATCH 1/2] dshow: fix return code when opening device
>>>
>>> Successfully opening a device altered the ret variable, making the
>>> function
>>> not cleanup properly and return an incorrect value for errors that
>>> happened
>>> afterwards.
>>> ---
>>>  libavdevice/dshow.c |   18 ++++++++----------
>>>  1 files changed, 8 insertions(+), 10 deletions(-)
>>>
>>> diff --git a/libavdevice/dshow.c b/libavdevice/dshow.c
>>> index ea01b2a..18a4ee9 100644
>>> --- a/libavdevice/dshow.c
>>> +++ b/libavdevice/dshow.c
>>> @@ -929,20 +929,18 @@ static int dshow_read_header(AVFormatContext
>>> *avctx)
>>>      }
>>>
>>>      if (ctx->device_name[VideoDevice]) {
>>> -        ret = dshow_open_device(avctx, devenum, VideoDevice);
>>> -        if (ret < 0)
>>> -            goto error;
>>> -        ret = dshow_add_device(avctx, VideoDevice);
>>> -        if (ret < 0)
>>> +        if ((r = dshow_open_device(avctx, devenum, VideoDevice)) < 0 ||
>>> +            (r = dshow_add_device(avctx, VideoDevice) < 0)) {
>>> +            ret = r;
>>>              goto error;
>>> +        }
>>
>>
>> Isn't this perfectly equivalent to the pre-existing code? The only
>> difference is that in case of success it sets the ret value to the
>> return code of dshow_open/add_device, which is finally returned (while
>> the other failures, e.g. if (!ctx->mutex), don't change the default
>> return value of 0, so won't cause a failure, is this intended?).
>
>
> The problem with the pre-existing code is that ret has been set to a zero
> value indicating success. If it fails after the above chunk of code it just
> does a goto error with a successful return (ret) value. That patch has a ')'
> wrong in 2 places though.
>
> When a device it already open in another app and you try to open it in
> ffmpeg what would happen is all would succeed until you hit the
> IMediaControl_Run statement which would fail and return success to the
> calling function since the error ret value was over written with a success
> value.

Patch updated.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0001-dshow-fix-return-code-when-opening-device.patch
Type: text/x-patch
Size: 1621 bytes
Desc: not available
URL: <http://ffmpeg.org/pipermail/ffmpeg-devel/attachments/20121206/c267f1e9/attachment.bin>