Quoting James Almer (2024-09-06 14:15:36)
> On 9/6/2024 8:56 AM, Anton Khirnov wrote:
> > Quoting James Almer (2024-08-31 18:31:14)
> >> static int demux_send(Demuxer *d, DemuxThreadContext *dt, DemuxStream
> >> *ds,
> >> AVPacket *pkt, unsigned flags)
> >> {
> >> InputFile *f = &d->f;
> >> - int ret;
> >> + int ret = 0;
> >>
> >> // pkt can be NULL only when flushing BSFs
> >> av_assert0(ds->bsf || pkt);
> >>
> >> + // a stream can only be disabled if it's needed by a group
> >
> > This makes no sense to me.
>
> I made av_read_frame() output packets for all the streams belonging to a
> group, even if some of those streams are not selected/used for an
> output. In the case of LCEVC groups, the normal scenario is to merge
> enhancement stream packets into the video one, with the enhancement
> stream not being used on its own by any output stream.
> DemuxStream->discard is used for this, where it can be 0 (not used by
> any output) when AVStream->discard is obviously going to be 1 for lavf
> to export packets for the stream.
>
> If a packet where DemuxStream->discard is 0 reaches this point, the only
> valid scenario is that it's part of a group.
>
> >
> >> + av_assert0(ds->nb_stream_groups || !ds->discard);
> >> +
> >> + // create a reference for the packet to be filtered by group bsfs
> >
> > What are "group bsfs"?
>
> Bsfs that are used by and all (or some) of a group's streams, as is the
> case of lcevc_merge, and stored in DemuxStreamGroup->bsf.
>
> >
> >> + if (pkt && ds->nb_stream_groups) {
> >> + av_packet_unref(dt->pkt_group_bsf);
> >> + ret = av_packet_ref(dt->pkt_group_bsf, pkt);
> >> + if (ret < 0)
> >> + return ret;
> >> + }
> >> +
> >> // send heartbeat for sub2video streams
> >> - if (d->pkt_heartbeat && pkt && pkt->pts != AV_NOPTS_VALUE) {
> >> + if (d->pkt_heartbeat && pkt && !ds->discard && pkt->pts !=
> >> AV_NOPTS_VALUE) {
> >
> > Random added checks for ds->discard are extremely confusing and tell me
> > you're overloading that poor field to mean something extremely
> > non-obvious.
>
> I added this here to make sure I'm not sending a heartbeat packet when
> handling a packet for a stream that's not used by any output on its own.
This is not the the only place where you're adding a check for discard,
and I strongly dislike that 'discard' now does not mean 'discard'
anymore.
Furthermore, I really dislike how invasive such an obscure feature is.
> >> + int j;
> >> +
> >> + for (j = 0; j < f->nb_streams; j++)
> >> + if (stg->streams[i] == f->streams[j]->st)
> >> + break;
> >> +
> >> + if (j == f->nb_streams)
> >> + return AVERROR_BUG;
> >
> > Isn't all this just "j = stg->streams[i]->index"?
>
> Is f->streams guaranteed to have the same streams as ic->streams? If so,
> probably. Will change then.
I guess you're right that it's better not to rely on it. Still, I'd
rather this was factored into a separate function.
--
Anton Khirnov
_______________________________________________
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".
