[FFmpeg-devel,3/3] avcodec/cbs_vp9: discard empty fragments

Submitted by James Almer on Oct. 30, 2018, 7:21 p.m.

Details

Message ID 20181030192151.6560-3-jamrial@gmail.com
State New
Headers show

Commit Message

James Almer Oct. 30, 2018, 7:21 p.m.
Signed-off-by: James Almer <jamrial@gmail.com>
---
 libavcodec/cbs_vp9.c | 3 +++
 1 file changed, 3 insertions(+)

Comments

Mark Thompson Oct. 30, 2018, 11:19 p.m.
On 30/10/18 19:21, James Almer wrote:
> Signed-off-by: James Almer <jamrial@gmail.com>
> ---
>  libavcodec/cbs_vp9.c | 3 +++
>  1 file changed, 3 insertions(+)
> 
> diff --git a/libavcodec/cbs_vp9.c b/libavcodec/cbs_vp9.c
> index 7498be4b73..812be4ddd5 100644
> --- a/libavcodec/cbs_vp9.c
> +++ b/libavcodec/cbs_vp9.c
> @@ -403,6 +403,9 @@ static int cbs_vp9_split_fragment(CodedBitstreamContext *ctx,
>      uint8_t superframe_header;
>      int err;
>  
> +    if (frag->data_size <= 0)
> +        return 0;
> +
>      // Last byte in the packet.
>      superframe_header = frag->data[frag->data_size - 1];
>  
> 

Seems fine, but why would an empty fragment appear here?

(Given that H.26[45] has pretty much the same check, maybe it should be in the generic code.)

Thanks,

- Mark
James Almer Oct. 30, 2018, 11:45 p.m.
On 10/30/2018 8:19 PM, Mark Thompson wrote:
> On 30/10/18 19:21, James Almer wrote:
>> Signed-off-by: James Almer <jamrial@gmail.com>
>> ---
>>  libavcodec/cbs_vp9.c | 3 +++
>>  1 file changed, 3 insertions(+)
>>
>> diff --git a/libavcodec/cbs_vp9.c b/libavcodec/cbs_vp9.c
>> index 7498be4b73..812be4ddd5 100644
>> --- a/libavcodec/cbs_vp9.c
>> +++ b/libavcodec/cbs_vp9.c
>> @@ -403,6 +403,9 @@ static int cbs_vp9_split_fragment(CodedBitstreamContext *ctx,
>>      uint8_t superframe_header;
>>      int err;
>>  
>> +    if (frag->data_size <= 0)
>> +        return 0;
>> +
>>      // Last byte in the packet.
>>      superframe_header = frag->data[frag->data_size - 1];
>>  
>>
> 
> Seems fine, but why would an empty fragment appear here?

I noticed this when i tried to reimplement the vp9 parser using cbs_vp9.
Libavformat passed dummy zero sized packets that resulted in cbs errors
trying to read the frame header.

> 
> (Given that H.26[45] has pretty much the same check, maybe it should be in the generic code.)

And AV1 has a while (size > 0), so yeah, it may be a good idea.

> 
> Thanks,
> 
> - Mark
> _______________________________________________
> ffmpeg-devel mailing list
> ffmpeg-devel@ffmpeg.org
> http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>
Mark Thompson Oct. 31, 2018, 9:55 p.m.
On 30/10/18 23:45, James Almer wrote:
> On 10/30/2018 8:19 PM, Mark Thompson wrote:
>> On 30/10/18 19:21, James Almer wrote:
>>> Signed-off-by: James Almer <jamrial@gmail.com>
>>> ---
>>>  libavcodec/cbs_vp9.c | 3 +++
>>>  1 file changed, 3 insertions(+)
>>>
>>> diff --git a/libavcodec/cbs_vp9.c b/libavcodec/cbs_vp9.c
>>> index 7498be4b73..812be4ddd5 100644
>>> --- a/libavcodec/cbs_vp9.c
>>> +++ b/libavcodec/cbs_vp9.c
>>> @@ -403,6 +403,9 @@ static int cbs_vp9_split_fragment(CodedBitstreamContext *ctx,
>>>      uint8_t superframe_header;
>>>      int err;
>>>  
>>> +    if (frag->data_size <= 0)
>>> +        return 0;
>>> +
>>>      // Last byte in the packet.
>>>      superframe_header = frag->data[frag->data_size - 1];
>>>  
>>>
>>
>> Seems fine, but why would an empty fragment appear here?
> 
> I noticed this when i tried to reimplement the vp9 parser using cbs_vp9.
> Libavformat passed dummy zero sized packets that resulted in cbs errors
> trying to read the frame header.

Seems like we'd prefer they didn't get this far (zero-sized packets are flush packets to decoders), but oh well.

>> (Given that H.26[45] has pretty much the same check, maybe it should be in the generic code.)
> 
> And AV1 has a while (size > 0), so yeah, it may be a good idea.

I don't mind - this patch LGTM, make it common if you like.

Thanks,

- Mark

Patch hide | download patch | download mbox

diff --git a/libavcodec/cbs_vp9.c b/libavcodec/cbs_vp9.c
index 7498be4b73..812be4ddd5 100644
--- a/libavcodec/cbs_vp9.c
+++ b/libavcodec/cbs_vp9.c
@@ -403,6 +403,9 @@  static int cbs_vp9_split_fragment(CodedBitstreamContext *ctx,
     uint8_t superframe_header;
     int err;
 
+    if (frag->data_size <= 0)
+        return 0;
+
     // Last byte in the packet.
     superframe_header = frag->data[frag->data_size - 1];