| Message ID | 20190608132145.10567-1-antonin.gouzer@gmail.com |
|---|---|
| State | New |
| Headers | show |
looks good to me if maintainer has no objections... On 08.06.2019, at 15:21, Antonin Gouzer <antonin.gouzer@gmail.com> wrote: > --- > Some codecs editors had miss interpreted the H264 standart and > have coded a wrong size in the SEI data. > size = SEI size + 1. > The SEI data is detected as "truncated" > Ex: https://drive.google.com/file/d/1cNtLwnfPnyJnYqE7OYhU3SCoLRtuXIUM/view?usp=sharing > Command: > ffprobe -print_format xml -show_frames -read_intervals %+0.04 truncated.h264 > This (simple) patch add the possibility to read this false truncated SEI data with the default stric_std_compliance or less. > The error remain logged in both cases. > > V2: Modifiy the patch for only the off by one values > > Thanks in advance ! > --- > libavcodec/h264_sei.c | 24 +++++++++++++++--------- > libavcodec/h264_sei.h | 2 +- > 2 files changed, 16 insertions(+), 10 deletions(-) > > diff --git a/libavcodec/h264_sei.c b/libavcodec/h264_sei.c > index d4eb9c0dab..7871cf87ed 100644 > --- a/libavcodec/h264_sei.c > +++ b/libavcodec/h264_sei.c > @@ -402,7 +402,7 @@ static int decode_alternative_transfer(H264SEIAlternativeTransfer *h, > } > > int ff_h264_sei_decode(H264SEIContext *h, GetBitContext *gb, > - const H264ParamSets *ps, void *logctx) > + const H264ParamSets *ps, AVCodecContext *avctx) > { > int master_ret = 0; > > @@ -425,27 +425,33 @@ int ff_h264_sei_decode(H264SEIContext *h, GetBitContext *gb, > } while (get_bits(gb, 8) == 255); > > if (size > get_bits_left(gb) / 8) { > - av_log(logctx, AV_LOG_ERROR, "SEI type %d size %d truncated at %d\n", > + > + if (size == get_bits_left(gb) / 8 + 1 && avctx->strict_std_compliance <= FF_COMPLIANCE_NORMAL){ > + av_log(avctx, AV_LOG_WARNING, "SEI type %d size %d truncated at %d\n", > type, 8*size, get_bits_left(gb)); > - return AVERROR_INVALIDDATA; > + } else { > + av_log(avctx, AV_LOG_ERROR, "SEI type %d size %d truncated at %d, data will not be read\n", > + type, 8*size, get_bits_left(gb)); > + return AVERROR_INVALIDDATA; > + } > } > next = get_bits_count(gb) + 8 * size; > > switch (type) { > case H264_SEI_TYPE_PIC_TIMING: // Picture timing SEI > - ret = decode_picture_timing(&h->picture_timing, gb, ps, logctx); > + ret = decode_picture_timing(&h->picture_timing, gb, ps, avctx); > break; > case H264_SEI_TYPE_USER_DATA_REGISTERED: > - ret = decode_registered_user_data(h, gb, logctx, size); > + ret = decode_registered_user_data(h, gb, avctx, size); > break; > case H264_SEI_TYPE_USER_DATA_UNREGISTERED: > - ret = decode_unregistered_user_data(&h->unregistered, gb, logctx, size); > + ret = decode_unregistered_user_data(&h->unregistered, gb, avctx, size); > break; > case H264_SEI_TYPE_RECOVERY_POINT: > - ret = decode_recovery_point(&h->recovery_point, gb, logctx); > + ret = decode_recovery_point(&h->recovery_point, gb, avctx); > break; > case H264_SEI_TYPE_BUFFERING_PERIOD: > - ret = decode_buffering_period(&h->buffering_period, gb, ps, logctx); > + ret = decode_buffering_period(&h->buffering_period, gb, ps, avctx); > break; > case H264_SEI_TYPE_FRAME_PACKING: > ret = decode_frame_packing_arrangement(&h->frame_packing, gb); > @@ -460,7 +466,7 @@ int ff_h264_sei_decode(H264SEIContext *h, GetBitContext *gb, > ret = decode_alternative_transfer(&h->alternative_transfer, gb); > break; > default: > - av_log(logctx, AV_LOG_DEBUG, "unknown SEI type %d\n", type); > + av_log(avctx, AV_LOG_DEBUG, "unknown SEI type %d\n", type); > } > if (ret < 0 && ret != AVERROR_PS_NOT_FOUND) > return ret; > diff --git a/libavcodec/h264_sei.h b/libavcodec/h264_sei.h > index a75c3aa175..32f3d67096 100644 > --- a/libavcodec/h264_sei.h > +++ b/libavcodec/h264_sei.h > @@ -190,7 +190,7 @@ typedef struct H264SEIContext { > struct H264ParamSets; > > int ff_h264_sei_decode(H264SEIContext *h, GetBitContext *gb, > - const struct H264ParamSets *ps, void *logctx); > + const struct H264ParamSets *ps, AVCodecContext *avctx); > > /** > * Reset SEI values at the beginning of the frame. > -- > 2.11.0 > > _______________________________________________ > 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 Sat, Jun 08, 2019 at 03:21:45PM +0200, Antonin Gouzer wrote: > --- > Some codecs editors had miss interpreted the H264 standart and > have coded a wrong size in the SEI data. > size = SEI size + 1. > The SEI data is detected as "truncated" This information does not end in the commit message, the result is a commit message thats a bit too terse, information wise. > Ex: https://drive.google.com/file/d/1cNtLwnfPnyJnYqE7OYhU3SCoLRtuXIUM/view?usp=sharing do you want to submit a fate test ? > Command: > ffprobe -print_format xml -show_frames -read_intervals %+0.04 truncated.h264 > This (simple) patch add the possibility to read this false truncated SEI data with the default stric_std_compliance or less. > The error remain logged in both cases. > > V2: Modifiy the patch for only the off by one values > > Thanks in advance ! > --- > libavcodec/h264_sei.c | 24 +++++++++++++++--------- > libavcodec/h264_sei.h | 2 +- > 2 files changed, 16 insertions(+), 10 deletions(-) > > diff --git a/libavcodec/h264_sei.c b/libavcodec/h264_sei.c > index d4eb9c0dab..7871cf87ed 100644 > --- a/libavcodec/h264_sei.c > +++ b/libavcodec/h264_sei.c > @@ -402,7 +402,7 @@ static int decode_alternative_transfer(H264SEIAlternativeTransfer *h, > } > > int ff_h264_sei_decode(H264SEIContext *h, GetBitContext *gb, > - const H264ParamSets *ps, void *logctx) > + const H264ParamSets *ps, AVCodecContext *avctx) > { you could split this change in a seperate patch, this would make git log slightly more readable > int master_ret = 0; > > @@ -425,27 +425,33 @@ int ff_h264_sei_decode(H264SEIContext *h, GetBitContext *gb, > } while (get_bits(gb, 8) == 255); > > if (size > get_bits_left(gb) / 8) { > - av_log(logctx, AV_LOG_ERROR, "SEI type %d size %d truncated at %d\n", > + > + if (size == get_bits_left(gb) / 8 + 1 && avctx->strict_std_compliance <= FF_COMPLIANCE_NORMAL){ > + av_log(avctx, AV_LOG_WARNING, "SEI type %d size %d truncated at %d\n", > type, 8*size, get_bits_left(gb)); > + } else { > + av_log(avctx, AV_LOG_ERROR, "SEI type %d size %d truncated at %d, data will not be read\n", > + type, 8*size, get_bits_left(gb)); > + return AVERROR_INVALIDDATA; > + } something is not right with the indention here thx [...]
On Sat, Jun 8, 2019 at 9:28 AM Antonin Gouzer <antonin.gouzer@gmail.com> wrote: > --- > Some codecs editors had miss interpreted the H264 standart and > have coded a wrong size in the SEI data. > size = SEI size + 1. > The SEI data is detected as "truncated" > Ex: > https://drive.google.com/file/d/1cNtLwnfPnyJnYqE7OYhU3SCoLRtuXIUM/view?usp=sharing > Command: > ffprobe -print_format xml -show_frames -read_intervals %+0.04 > truncated.h264 > This (simple) patch add the possibility to read this false truncated SEI > data with the default stric_std_compliance or less. > The error remain logged in both cases. > > V2: Modifiy the patch for only the off by one values > > Thanks in advance ! > --- > libavcodec/h264_sei.c | 24 +++++++++++++++--------- > libavcodec/h264_sei.h | 2 +- > 2 files changed, 16 insertions(+), 10 deletions(-) > > diff --git a/libavcodec/h264_sei.c b/libavcodec/h264_sei.c > index d4eb9c0dab..7871cf87ed 100644 > --- a/libavcodec/h264_sei.c > +++ b/libavcodec/h264_sei.c > @@ -402,7 +402,7 @@ static int > decode_alternative_transfer(H264SEIAlternativeTransfer *h, > } > > int ff_h264_sei_decode(H264SEIContext *h, GetBitContext *gb, > - const H264ParamSets *ps, void *logctx) > + const H264ParamSets *ps, AVCodecContext *avctx) > { > int master_ret = 0; > > This may be a minor note, but i don't think it's worth to check for std compliance within a *parsing* function: in my opinion it would make more sense to check in the calling function, and always allow for truncated sei, and error out only if (avctx->error | AV_ERROR_EXPLODE) is set (sorry going by memory here)
diff --git a/libavcodec/h264_sei.c b/libavcodec/h264_sei.c index d4eb9c0dab..7871cf87ed 100644 --- a/libavcodec/h264_sei.c +++ b/libavcodec/h264_sei.c @@ -402,7 +402,7 @@ static int decode_alternative_transfer(H264SEIAlternativeTransfer *h, } int ff_h264_sei_decode(H264SEIContext *h, GetBitContext *gb, - const H264ParamSets *ps, void *logctx) + const H264ParamSets *ps, AVCodecContext *avctx) { int master_ret = 0; @@ -425,27 +425,33 @@ int ff_h264_sei_decode(H264SEIContext *h, GetBitContext *gb, } while (get_bits(gb, 8) == 255); if (size > get_bits_left(gb) / 8) { - av_log(logctx, AV_LOG_ERROR, "SEI type %d size %d truncated at %d\n", + + if (size == get_bits_left(gb) / 8 + 1 && avctx->strict_std_compliance <= FF_COMPLIANCE_NORMAL){ + av_log(avctx, AV_LOG_WARNING, "SEI type %d size %d truncated at %d\n", type, 8*size, get_bits_left(gb)); - return AVERROR_INVALIDDATA; + } else { + av_log(avctx, AV_LOG_ERROR, "SEI type %d size %d truncated at %d, data will not be read\n", + type, 8*size, get_bits_left(gb)); + return AVERROR_INVALIDDATA; + } } next = get_bits_count(gb) + 8 * size; switch (type) { case H264_SEI_TYPE_PIC_TIMING: // Picture timing SEI - ret = decode_picture_timing(&h->picture_timing, gb, ps, logctx); + ret = decode_picture_timing(&h->picture_timing, gb, ps, avctx); break; case H264_SEI_TYPE_USER_DATA_REGISTERED: - ret = decode_registered_user_data(h, gb, logctx, size); + ret = decode_registered_user_data(h, gb, avctx, size); break; case H264_SEI_TYPE_USER_DATA_UNREGISTERED: - ret = decode_unregistered_user_data(&h->unregistered, gb, logctx, size); + ret = decode_unregistered_user_data(&h->unregistered, gb, avctx, size); break; case H264_SEI_TYPE_RECOVERY_POINT: - ret = decode_recovery_point(&h->recovery_point, gb, logctx); + ret = decode_recovery_point(&h->recovery_point, gb, avctx); break; case H264_SEI_TYPE_BUFFERING_PERIOD: - ret = decode_buffering_period(&h->buffering_period, gb, ps, logctx); + ret = decode_buffering_period(&h->buffering_period, gb, ps, avctx); break; case H264_SEI_TYPE_FRAME_PACKING: ret = decode_frame_packing_arrangement(&h->frame_packing, gb); @@ -460,7 +466,7 @@ int ff_h264_sei_decode(H264SEIContext *h, GetBitContext *gb, ret = decode_alternative_transfer(&h->alternative_transfer, gb); break; default: - av_log(logctx, AV_LOG_DEBUG, "unknown SEI type %d\n", type); + av_log(avctx, AV_LOG_DEBUG, "unknown SEI type %d\n", type); } if (ret < 0 && ret != AVERROR_PS_NOT_FOUND) return ret; diff --git a/libavcodec/h264_sei.h b/libavcodec/h264_sei.h index a75c3aa175..32f3d67096 100644 --- a/libavcodec/h264_sei.h +++ b/libavcodec/h264_sei.h @@ -190,7 +190,7 @@ typedef struct H264SEIContext { struct H264ParamSets; int ff_h264_sei_decode(H264SEIContext *h, GetBitContext *gb, - const struct H264ParamSets *ps, void *logctx); + const struct H264ParamSets *ps, AVCodecContext *avctx); /** * Reset SEI values at the beginning of the frame.