| Message ID | 20181030192151.6560-3-jamrial@gmail.com |
|---|---|
| State | New |
| Headers | show |
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
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 >
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
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];
Signed-off-by: James Almer <jamrial@gmail.com> --- libavcodec/cbs_vp9.c | 3 +++ 1 file changed, 3 insertions(+)