Andreas Rheinhardt:
> Andreas Rheinhardt:
>> There is no check for whether these supposedly redundant PPS
>> are actually redundant. One could check via memcmp which would
>> work in practice* (because all content buffers are initially
>> zero-allocated), but this is not portable as compilers may
>> trash padding inside structures as they wish.
>>
>> In case the PPS is not really redundant the output is garbage.
>> This happens with several files from the FATE-suite. E.g.
>> h264-conformance/CVCANLMA2_Sony_C.jsv doesn't decode correctly
>> any more, whereas h264-conformance/CABA3_TOSHIBA_E.264 even
>> fails in ff_cbs_write_packet(), because the inferred value
>> of num_ref_idx_l0_active_minus1 mismatches with the value set
>> in the slice (this happens when num_ref_idx_l0_default_active_minus1
>> changes in the PPS; the value in the slice header is inferred from
>> the original PPS's num_ref_idx_l0_default_active_minus1).
>>
>> *: Unless slice_group_id is used, i.e. unless slice_group_map_type
>> is six.
>>
>> Signed-off-by: Andreas Rheinhardt <andreas.rheinha...@outlook.com>
>> ---
>> libavcodec/h264_redundant_pps_bsf.c | 11 -----------
>> 1 file changed, 11 deletions(-)
>>
>> diff --git a/libavcodec/h264_redundant_pps_bsf.c
>> b/libavcodec/h264_redundant_pps_bsf.c
>> index f8bab1f109..df9a88a705 100644
>> --- a/libavcodec/h264_redundant_pps_bsf.c
>> +++ b/libavcodec/h264_redundant_pps_bsf.c
>> @@ -80,26 +80,15 @@ static int
>> h264_redundant_pps_update_fragment(AVBSFContext *bsf,
>> CodedBitstreamFragment *au)
>> {
>> H264RedundantPPSContext *ctx = bsf->priv_data;
>> - int au_has_sps;
>> int err, i;
>>
>> - au_has_sps = 0;
>> for (i = 0; i < au->nb_units; i++) {
>> CodedBitstreamUnit *nal = &au->units[i];
>>
>> - if (nal->type == H264_NAL_SPS)
>> - au_has_sps = 1;
>> if (nal->type == H264_NAL_PPS) {
>> err = h264_redundant_pps_fixup_pps(ctx, nal);
>> if (err < 0)
>> return err;
>> - if (!au_has_sps) {
>> - av_log(bsf, AV_LOG_VERBOSE, "Deleting redundant PPS "
>> - "at %"PRId64".\n", pkt->pts);
>> - ff_cbs_delete_unit(au, i);
>> - i--;
>> - continue;
>> - }
>> }
>> if (nal->type == H264_NAL_SLICE ||
>> nal->type == H264_NAL_IDR_SLICE) {
>
> I just noticed that the documentation contains the sentence "A new
> single global PPS is created, and all of the redundant PPSs within the
> stream are removed". As the observations in the commit message show,
> this is just not true because every PPS in an access unit without SPS is
> considered redundant, making this filter dangerous. So I would simply
> delete this sentence from the documentation. Is everyone ok with this?
>
Will apply this patchset tomorrow unless there are objections.
- Andreas
_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
