| Message ID | bf15fa33c739b0a4b4d32bedeec6f326e3b175e0.1652120184.git.ffmpegagent@gmail.com |
|---|---|
| State | Accepted |
| Commit | c534d9f72a89542ed639071b1ae15893aadf1f18 |
| Headers | show |
| Series | lavc/videotoolboxdec: improve HEVC stream compatibility | expand |
| Context | Check | Description |
|---|---|---|
| yinshiyou/make_loongarch64 | success | Make finished |
| yinshiyou/make_fate_loongarch64 | success | Make fate finished |
| andriy/make_x86 | success | Make finished |
| andriy/make_fate_x86 | success | Make fate finished |
rcombs: > From: rcombs <rcombs@rcombs.me> > > The VideoToolbox hwaccel needs the entire NAL (including the stop bit), > but ff_h2645_packet_split may remove it. Detect this case by looking for > bit counts divisible by 8 and insert a stop-bit-only 0x80 byte. > > Signed-off-by: rcombs <rcombs@rcombs.me> > --- > libavcodec/h264_ps.c | 8 ++++++++ > 1 file changed, 8 insertions(+) > > diff --git a/libavcodec/h264_ps.c b/libavcodec/h264_ps.c > index 051f06692c..e16da68dec 100644 > --- a/libavcodec/h264_ps.c > +++ b/libavcodec/h264_ps.c > @@ -351,6 +351,10 @@ int ff_h264_decode_seq_parameter_set(GetBitContext *gb, AVCodecContext *avctx, > } > memcpy(sps->data, gb->buffer, sps->data_size); > > + // Re-add the removed stop bit (may be used by hwaccels). > + if (!(gb->size_in_bits & 7) && sps->data_size < sizeof(sps->data)) > + sps->data[sps->data_size++] = 0x80; > + > profile_idc = get_bits(gb, 8); > constraint_set_flags |= get_bits1(gb) << 0; // constraint_set0_flag > constraint_set_flags |= get_bits1(gb) << 1; // constraint_set1_flag > @@ -775,6 +779,10 @@ int ff_h264_decode_picture_parameter_set(GetBitContext *gb, AVCodecContext *avct > } > memcpy(pps->data, gb->buffer, pps->data_size); > > + // Re-add the removed stop bit (may be used by hwaccels). > + if (!(bit_length & 7) && pps->data_size < sizeof(pps->data)) > + pps->data[pps->data_size++] = 0x80; > + > pps->sps_id = get_ue_golomb_31(gb); > if ((unsigned)pps->sps_id >= MAX_SPS_COUNT || > !ps->sps_list[pps->sps_id]) { Modifying these functions affects more than just VideoToolbox. Are you sure that the other users of this file are ok with this change? - Andreas
> On May 9, 2022, at 23:26, Andreas Rheinhardt <andreas.rheinhardt@outlook.com> wrote: > > rcombs: >> From: rcombs <rcombs@rcombs.me> >> >> The VideoToolbox hwaccel needs the entire NAL (including the stop bit), >> but ff_h2645_packet_split may remove it. Detect this case by looking for >> bit counts divisible by 8 and insert a stop-bit-only 0x80 byte. >> >> Signed-off-by: rcombs <rcombs@rcombs.me> >> --- >> libavcodec/h264_ps.c | 8 ++++++++ >> 1 file changed, 8 insertions(+) >> >> diff --git a/libavcodec/h264_ps.c b/libavcodec/h264_ps.c >> index 051f06692c..e16da68dec 100644 >> --- a/libavcodec/h264_ps.c >> +++ b/libavcodec/h264_ps.c >> @@ -351,6 +351,10 @@ int ff_h264_decode_seq_parameter_set(GetBitContext *gb, AVCodecContext *avctx, >> } >> memcpy(sps->data, gb->buffer, sps->data_size); >> >> + // Re-add the removed stop bit (may be used by hwaccels). >> + if (!(gb->size_in_bits & 7) && sps->data_size < sizeof(sps->data)) >> + sps->data[sps->data_size++] = 0x80; >> + >> profile_idc = get_bits(gb, 8); >> constraint_set_flags |= get_bits1(gb) << 0; // constraint_set0_flag >> constraint_set_flags |= get_bits1(gb) << 1; // constraint_set1_flag >> @@ -775,6 +779,10 @@ int ff_h264_decode_picture_parameter_set(GetBitContext *gb, AVCodecContext *avct >> } >> memcpy(pps->data, gb->buffer, pps->data_size); >> >> + // Re-add the removed stop bit (may be used by hwaccels). >> + if (!(bit_length & 7) && pps->data_size < sizeof(pps->data)) >> + pps->data[pps->data_size++] = 0x80; >> + >> pps->sps_id = get_ue_golomb_31(gb); >> if ((unsigned)pps->sps_id >= MAX_SPS_COUNT || >> !ps->sps_list[pps->sps_id]) { > > Modifying these functions affects more than just VideoToolbox. Are you > sure that the other users of this file are ok with this change? > > - Andreas It passes FATE, and only adds a byte containing the stop bit (which would be present prior to this commit in 7 of 8 possible cases), so I believe it should be fine (and conceptually including the stop bit makes more sense IMO; being inconsistent about it like this is wacky). > _______________________________________________ > ffmpeg-devel mailing list > ffmpeg-devel@ffmpeg.org <mailto:ffmpeg-devel@ffmpeg.org> > https://ffmpeg.org/mailman/listinfo/ffmpeg-devel <https://ffmpeg.org/mailman/listinfo/ffmpeg-devel> > > To unsubscribe, visit link above, or email > ffmpeg-devel-request@ffmpeg.org <mailto:ffmpeg-devel-request@ffmpeg.org> with subject "unsubscribe".
diff --git a/libavcodec/h264_ps.c b/libavcodec/h264_ps.c index 051f06692c..e16da68dec 100644 --- a/libavcodec/h264_ps.c +++ b/libavcodec/h264_ps.c @@ -351,6 +351,10 @@ int ff_h264_decode_seq_parameter_set(GetBitContext *gb, AVCodecContext *avctx, } memcpy(sps->data, gb->buffer, sps->data_size); + // Re-add the removed stop bit (may be used by hwaccels). + if (!(gb->size_in_bits & 7) && sps->data_size < sizeof(sps->data)) + sps->data[sps->data_size++] = 0x80; + profile_idc = get_bits(gb, 8); constraint_set_flags |= get_bits1(gb) << 0; // constraint_set0_flag constraint_set_flags |= get_bits1(gb) << 1; // constraint_set1_flag @@ -775,6 +779,10 @@ int ff_h264_decode_picture_parameter_set(GetBitContext *gb, AVCodecContext *avct } memcpy(pps->data, gb->buffer, pps->data_size); + // Re-add the removed stop bit (may be used by hwaccels). + if (!(bit_length & 7) && pps->data_size < sizeof(pps->data)) + pps->data[pps->data_size++] = 0x80; + pps->sps_id = get_ue_golomb_31(gb); if ((unsigned)pps->sps_id >= MAX_SPS_COUNT || !ps->sps_list[pps->sps_id]) {