| Message ID | 20211203080920.1948453-1-fei.w.wang@intel.com |
|---|---|
| State | New |
| Headers | show |
| Series | [FFmpeg-devel,v1] avcodec/av1_parser: ensure only one show frame packed data parsered | expand |
| Context | Check | Description |
|---|---|---|
| andriy/make_x86 | success | Make finished |
| andriy/make_fate_x86 | success | Make fate finished |
| andriy/make_ppc | success | Make finished |
| andriy/make_fate_ppc | success | Make fate finished |
On Fri, Dec 03, 2021 at 04:09:20PM +0800, Fei Wang wrote: > Split packed data when it contains multiple show frames in some > non-standard bitstream. This can benefit downstream decoder which can > decode continuously instead of interrupt with unexpected error. > > Signed-off-by: Fei Wang <fei.w.wang@intel.com> > --- > libavcodec/av1_parser.c | 19 +++++++++++++++---- > 1 file changed, 15 insertions(+), 4 deletions(-) This results in a infinite loop endlessly displaying [av1 @ 0x55f62c2a6a00] Failed to parse temporal unit. [av1 @ 0x55f62c2a6a00] Invalid OBU length: 1071323, but only 768 bytes remaining in fragment. [av1 @ 0x55f62c2a6a00] Failed to parse temporal unit. [av1 @ 0x55f62c2a6a00] Invalid OBU length: 1071323, but only 768 bytes remaining in fragment. [av1 @ 0x55f62c2a6a00] Failed to parse temporal unit. [av1 @ 0x55f62c2a6a00] Invalid OBU length: 1071323, but only 768 bytes remaining in fragment. [av1 @ 0x55f62c2a6a00] Failed to parse temporal unit. [av1 @ 0x55f62c2a6a00] Invalid OBU length: 1071323, but only 768 bytes remaining in fragment. [av1 @ 0x55f62c2a6a00] Failed to parse temporal unit. [av1 @ 0x55f62c2a6a00] Invalid OBU length: 1071323, but only 768 bytes remaining in fragment. [av1 @ 0x55f62c2a6a00] Failed to parse temporal unit. [av1 @ 0x55f62c2a6a00] Invalid OBU length: 1071323, but only 768 bytes remaining in fragment. [av1 @ 0x55f62c2a6a00] Failed to parse temporal unit. [av1 @ 0x55f62c2a6a00] Invalid OBU length: 1071323, but only 768 bytes remaining in fragment. [av1 @ 0x55f62c2a6a00] Failed to parse temporal unit. i will send you the testsample privatly thx [...]
On Fri, Dec 3, 2021 at 5:12 AM Fei Wang <fei.w.wang@intel.com> wrote: > Split packed data when it contains multiple show frames in some > non-standard bitstream. This can benefit downstream decoder which can > decode continuously instead of interrupt with unexpected error. > > Signed-off-by: Fei Wang <fei.w.wang@intel.com> > --- > libavcodec/av1_parser.c | 19 +++++++++++++++---- > 1 file changed, 15 insertions(+), 4 deletions(-) > > diff --git a/libavcodec/av1_parser.c b/libavcodec/av1_parser.c > index d2dfdb3580..d6f5cace4b 100644 > --- a/libavcodec/av1_parser.c > +++ b/libavcodec/av1_parser.c > @@ -59,11 +59,10 @@ static int av1_parser_parse(AVCodecParserContext *ctx, > const CodedBitstreamAV1Context *av1 = s->cbc->priv_data; > const AV1RawSequenceHeader *seq; > const AV1RawColorConfig *color; > + int pic_found = 0; > + int next = 0; > int ret; > > - *out_data = data; > - *out_size = size; > - > ctx->key_frame = -1; > ctx->pict_type = AV_PICTURE_TYPE_NONE; > ctx->picture_structure = AV_PICTURE_STRUCTURE_UNKNOWN; > @@ -100,6 +99,8 @@ static int av1_parser_parse(AVCodecParserContext *ctx, > const AV1RawOBU *obu = unit->content; > const AV1RawFrameHeader *frame; > > + next += unit->data_size; > + > if (unit->type == AV1_OBU_FRAME) > frame = &obu->obu.frame.header; > else if (unit->type == AV1_OBU_FRAME_HEADER) > @@ -113,6 +114,12 @@ static int av1_parser_parse(AVCodecParserContext *ctx, > if (!frame->show_frame && !frame->show_existing_frame) > continue; > > + /* split data if it contains multi show frames */ > + if (pic_found) { > + next -= unit->data_size; > + break; > + } > + > ctx->width = frame->frame_width_minus_1 + 1; > ctx->height = frame->frame_height_minus_1 + 1; > > @@ -131,8 +138,12 @@ static int av1_parser_parse(AVCodecParserContext *ctx, > break; > } > ctx->picture_structure = AV_PICTURE_STRUCTURE_FRAME; > + pic_found = 1; > } > > + *out_size = next; > + *out_data = data; > + > switch (av1->bit_depth) { > case 8: > ctx->format = color->mono_chrome ? AV_PIX_FMT_GRAY8 > @@ -171,7 +182,7 @@ end: > > s->cbc->log_ctx = NULL; > > - return size; > + return next; > } > > static const CodedBitstreamUnitType decompose_unit_types[] = { > -- > 2.25.1 > > No, the parser must not split the packets. We used to do this with the VP9 parser and the result was the creation of broken output files. What you could do instead is autoinsert the frame_split BSF in the decoder, same as we're doing for VP9. It should have the same effect.
On Fri, 2021-12-03 at 11:51 +0100, Michael Niedermayer wrote: > On Fri, Dec 03, 2021 at 04:09:20PM +0800, Fei Wang wrote: > > Split packed data when it contains multiple show frames in some > > non-standard bitstream. This can benefit downstream decoder which > > can > > decode continuously instead of interrupt with unexpected error. > > > > Signed-off-by: Fei Wang <fei.w.wang@intel.com> > > --- > > libavcodec/av1_parser.c | 19 +++++++++++++++---- > > 1 file changed, 15 insertions(+), 4 deletions(-) > > This results in a infinite loop > > endlessly displaying > [av1 @ 0x55f62c2a6a00] Failed to parse temporal unit. > [av1 @ 0x55f62c2a6a00] Invalid OBU length: 1071323, but only 768 > bytes remaining in fragment. > [av1 @ 0x55f62c2a6a00] Failed to parse temporal unit. > [av1 @ 0x55f62c2a6a00] Invalid OBU length: 1071323, but only 768 > bytes remaining in fragment. > [av1 @ 0x55f62c2a6a00] Failed to parse temporal unit. > [av1 @ 0x55f62c2a6a00] Invalid OBU length: 1071323, but only 768 > bytes remaining in fragment. > [av1 @ 0x55f62c2a6a00] Failed to parse temporal unit. > [av1 @ 0x55f62c2a6a00] Invalid OBU length: 1071323, but only 768 > bytes remaining in fragment. > [av1 @ 0x55f62c2a6a00] Failed to parse temporal unit. > [av1 @ 0x55f62c2a6a00] Invalid OBU length: 1071323, but only 768 > bytes remaining in fragment. > [av1 @ 0x55f62c2a6a00] Failed to parse temporal unit. > [av1 @ 0x55f62c2a6a00] Invalid OBU length: 1071323, but only 768 > bytes remaining in fragment. > [av1 @ 0x55f62c2a6a00] Failed to parse temporal unit. > [av1 @ 0x55f62c2a6a00] Invalid OBU length: 1071323, but only 768 > bytes remaining in fragment. > [av1 @ 0x55f62c2a6a00] Failed to parse temporal unit. Thanks Michael to catch this. Will check it soon. Fei Thanks > > i will send you the testsample privatly > > thx > > [...] > _______________________________________________ > ffmpeg-devel mailing list > ffmpeg-devel@ffmpeg.org > https://ffmpeg.org/mailman/listinfo/ffmpeg-devel > > To unsubscribe, visit link above, or email > ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".
On Fri, 2021-12-03 at 09:36 -0300, James Almer wrote: > On Fri, Dec 3, 2021 at 5:12 AM Fei Wang <fei.w.wang@intel.com> wrote: > > > Split packed data when it contains multiple show frames in some > > non-standard bitstream. This can benefit downstream decoder which > > can > > decode continuously instead of interrupt with unexpected error. > > > > Signed-off-by: Fei Wang <fei.w.wang@intel.com> > > --- > > libavcodec/av1_parser.c | 19 +++++++++++++++---- > > 1 file changed, 15 insertions(+), 4 deletions(-) > > > > diff --git a/libavcodec/av1_parser.c b/libavcodec/av1_parser.c > > index d2dfdb3580..d6f5cace4b 100644 > > --- a/libavcodec/av1_parser.c > > +++ b/libavcodec/av1_parser.c > > @@ -59,11 +59,10 @@ static int > > av1_parser_parse(AVCodecParserContext *ctx, > > const CodedBitstreamAV1Context *av1 = s->cbc->priv_data; > > const AV1RawSequenceHeader *seq; > > const AV1RawColorConfig *color; > > + int pic_found = 0; > > + int next = 0; > > int ret; > > > > - *out_data = data; > > - *out_size = size; > > - > > ctx->key_frame = -1; > > ctx->pict_type = AV_PICTURE_TYPE_NONE; > > ctx->picture_structure = AV_PICTURE_STRUCTURE_UNKNOWN; > > @@ -100,6 +99,8 @@ static int av1_parser_parse(AVCodecParserContext > > *ctx, > > const AV1RawOBU *obu = unit->content; > > const AV1RawFrameHeader *frame; > > > > + next += unit->data_size; > > + > > if (unit->type == AV1_OBU_FRAME) > > frame = &obu->obu.frame.header; > > else if (unit->type == AV1_OBU_FRAME_HEADER) > > @@ -113,6 +114,12 @@ static int > > av1_parser_parse(AVCodecParserContext *ctx, > > if (!frame->show_frame && !frame->show_existing_frame) > > continue; > > > > + /* split data if it contains multi show frames */ > > + if (pic_found) { > > + next -= unit->data_size; > > + break; > > + } > > + > > ctx->width = frame->frame_width_minus_1 + 1; > > ctx->height = frame->frame_height_minus_1 + 1; > > > > @@ -131,8 +138,12 @@ static int > > av1_parser_parse(AVCodecParserContext *ctx, > > break; > > } > > ctx->picture_structure = AV_PICTURE_STRUCTURE_FRAME; > > + pic_found = 1; > > } > > > > + *out_size = next; > > + *out_data = data; > > + > > switch (av1->bit_depth) { > > case 8: > > ctx->format = color->mono_chrome ? AV_PIX_FMT_GRAY8 > > @@ -171,7 +182,7 @@ end: > > > > s->cbc->log_ctx = NULL; > > > > - return size; > > + return next; > > } > > > > static const CodedBitstreamUnitType decompose_unit_types[] = { > > -- > > 2.25.1 > > > > > No, the parser must not split the packets. We used to do this with > the VP9 > parser and the result was the creation of broken output files. Any more information on failed case for vp9? I tested this patch by decoding several av1 bitstream, all looks good. And I checked wm4's change on vp9/vp9_parser, the commit only shows it benefits on multi-thread decoding. So I guess if vp9 failed case is related to multi-thread decodeing? If yes, then insert frame_split bsf in av1 decoder will be a better choice. Even av1 decoder doesn't support multi-threading decode now, it can avoid issue if it is supported someday. > > What you could do instead is autoinsert the frame_split BSF in the > decoder, > same as we're doing for VP9. It should have the same effect. > _______________________________________________ > ffmpeg-devel mailing list > ffmpeg-devel@ffmpeg.org > https://ffmpeg.org/mailman/listinfo/ffmpeg-devel > > To unsubscribe, visit link above, or email > ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".
diff --git a/libavcodec/av1_parser.c b/libavcodec/av1_parser.c index d2dfdb3580..d6f5cace4b 100644 --- a/libavcodec/av1_parser.c +++ b/libavcodec/av1_parser.c @@ -59,11 +59,10 @@ static int av1_parser_parse(AVCodecParserContext *ctx, const CodedBitstreamAV1Context *av1 = s->cbc->priv_data; const AV1RawSequenceHeader *seq; const AV1RawColorConfig *color; + int pic_found = 0; + int next = 0; int ret; - *out_data = data; - *out_size = size; - ctx->key_frame = -1; ctx->pict_type = AV_PICTURE_TYPE_NONE; ctx->picture_structure = AV_PICTURE_STRUCTURE_UNKNOWN; @@ -100,6 +99,8 @@ static int av1_parser_parse(AVCodecParserContext *ctx, const AV1RawOBU *obu = unit->content; const AV1RawFrameHeader *frame; + next += unit->data_size; + if (unit->type == AV1_OBU_FRAME) frame = &obu->obu.frame.header; else if (unit->type == AV1_OBU_FRAME_HEADER) @@ -113,6 +114,12 @@ static int av1_parser_parse(AVCodecParserContext *ctx, if (!frame->show_frame && !frame->show_existing_frame) continue; + /* split data if it contains multi show frames */ + if (pic_found) { + next -= unit->data_size; + break; + } + ctx->width = frame->frame_width_minus_1 + 1; ctx->height = frame->frame_height_minus_1 + 1; @@ -131,8 +138,12 @@ static int av1_parser_parse(AVCodecParserContext *ctx, break; } ctx->picture_structure = AV_PICTURE_STRUCTURE_FRAME; + pic_found = 1; } + *out_size = next; + *out_data = data; + switch (av1->bit_depth) { case 8: ctx->format = color->mono_chrome ? AV_PIX_FMT_GRAY8 @@ -171,7 +182,7 @@ end: s->cbc->log_ctx = NULL; - return size; + return next; } static const CodedBitstreamUnitType decompose_unit_types[] = {
Split packed data when it contains multiple show frames in some non-standard bitstream. This can benefit downstream decoder which can decode continuously instead of interrupt with unexpected error. Signed-off-by: Fei Wang <fei.w.wang@intel.com> --- libavcodec/av1_parser.c | 19 +++++++++++++++---- 1 file changed, 15 insertions(+), 4 deletions(-)