diff mbox series

[FFmpeg-devel,3/3] avcodec/qpeg: mark the init function as thread-safe

Message ID 20200417031008.2131-3-jamrial@gmail.com
State New
Headers show
Series [FFmpeg-devel,1/3] avcodec/qpeg: remove an unnecessary intermediary AVFrame | expand

Checks

Context Check Description
andriy/default pending
andriy/make success Make finished
andriy/make_fate success Make fate finished

Commit Message

James Almer April 17, 2020, 3:10 a.m. UTC 
Signed-off-by: James Almer <jamrial@gmail.com>
---
 libavcodec/qpeg.c | 1 +
 1 file changed, 1 insertion(+)

Comments

Anton Khirnov April 20, 2020, 1:30 p.m. UTC | #1 
Quoting James Almer (2020-04-17 05:10:08)
> Signed-off-by: James Almer <jamrial@gmail.com>
> ---
>  libavcodec/qpeg.c | 1 +
>  1 file changed, 1 insertion(+)

Looks good.
Could have INIT_CLEANUP too, it seems.
James Almer April 20, 2020, 1:39 p.m. UTC | #2 
On 4/20/2020 10:30 AM, Anton Khirnov wrote:
> Quoting James Almer (2020-04-17 05:10:08)
>> Signed-off-by: James Almer <jamrial@gmail.com>
>> ---
>>  libavcodec/qpeg.c | 1 +
>>  1 file changed, 1 insertion(+)
> 
> Looks good.
> Could have INIT_CLEANUP too, it seems.

No, since the only way for init() to fail now is if allocating a->ref
fails, and if that happens then there's nothing to free.

Will apply, thanks.
Anton Khirnov April 20, 2020, 4:28 p.m. UTC | #3 
Quoting James Almer (2020-04-20 15:39:19)
> On 4/20/2020 10:30 AM, Anton Khirnov wrote:
> > Quoting James Almer (2020-04-17 05:10:08)
> >> Signed-off-by: James Almer <jamrial@gmail.com>
> >> ---
> >>  libavcodec/qpeg.c | 1 +
> >>  1 file changed, 1 insertion(+)
> > 
> > Looks good.
> > Could have INIT_CLEANUP too, it seems.
> 
> No, since the only way for init() to fail now is if allocating a->ref
> fails, and if that happens then there's nothing to free.

Which means it's trivially INIT_CLEANUP.
I believe the intent was to gradually mark all the codecs as
INIT_CLEANUP and then drop the flag so they all behave the same.

Anyway, that's just a nit, feel free to ignore.
James Almer April 20, 2020, 4:46 p.m. UTC | #4 
On 4/20/2020 1:28 PM, Anton Khirnov wrote:
> Quoting James Almer (2020-04-20 15:39:19)
>> On 4/20/2020 10:30 AM, Anton Khirnov wrote:
>>> Quoting James Almer (2020-04-17 05:10:08)
>>>> Signed-off-by: James Almer <jamrial@gmail.com>
>>>> ---
>>>>  libavcodec/qpeg.c | 1 +
>>>>  1 file changed, 1 insertion(+)
>>>
>>> Looks good.
>>> Could have INIT_CLEANUP too, it seems.
>>
>> No, since the only way for init() to fail now is if allocating a->ref
>> fails, and if that happens then there's nothing to free.
> 
> Which means it's trivially INIT_CLEANUP.
> I believe the intent was to gradually mark all the codecs as
> INIT_CLEANUP and then drop the flag so they all behave the same.

Never interpreted it that way myself, since in decoders where it's not
needed it results in a call to close() for no gain, but ok, added and
pushed.

Thanks.

> 
> Anyway, that's just a nit, feel free to ignore.
>
diff mbox series

Patch

diff --git a/libavcodec/qpeg.c b/libavcodec/qpeg.c
index 22afd9fa81..cc3bfda4aa 100644
--- a/libavcodec/qpeg.c
+++ b/libavcodec/qpeg.c
@@ -366,4 +366,5 @@  AVCodec ff_qpeg_decoder = {
     .decode         = decode_frame,
     .flush          = decode_flush,
     .capabilities   = AV_CODEC_CAP_DR1,
+    .caps_internal  = FF_CODEC_CAP_INIT_THREADSAFE,
 };