Re: [FFmpeg-devel] [PATCH 13/13] fftools/ffmpeg_demux: merge streams in a LCEVC stream group

[James Almer]

On 9/6/2024 10:33 AM, Anton Khirnov wrote:

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;

       av_assert0(ds->bsf || pkt);

// pkt can be NULL only when flushing BSFs + // 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,

Because it's the only code i don't want to trigger for non-standalone output packets.

and I strongly dislike that 'discard' now does not mean 'discard'
anymore.

No, the packet is discarded in the end, and never makes anywhere on its own unless it's used by an output. DemuxStream->discard is in fact the one field where you can check if a stream is effectively disabled, and that doesn't change with this patch. What changes is that lavf may now output packets not used by any output (AVStream->discard being 0), and the code needs to be aware of this.

Furthermore, I really dislike how invasive such an obscure feature is.

ffmpeg -i input.mp4 output.mp4


ffmpeg -i input.mp4 -c:v copy -dn -map 0:0 output.mp4


ffmpeg -i input.mp4 -c:d copy -vn -map 0:1 output.mp4


ffmpeg -i input.mp4 -c:v copy -c:d copy -map 0 output.mp4

Ideally, lavf would export the LCEVC stream payload as packet side data in the video's stream, but that's apparently only possible with mov/mp4 and maybe matroska, not TS. Hence a Stream Group is used and everything left to the caller. That said, is this so invasive? This patch adds DemuxStreamGroup/InputStreamGroup, cleanly initializes them like we already do with DemuxStream/InputStream, and then factorizes the bsf packet loop so it can be used for the stream bsf and the group bsf. The only truly big change is making libavformat output streams that are not strictly used by an output, which is achieved by making AVStream->discard not be set, and keep relying on DemuxStream->discard to know what stream is disabled or not. You can try remuxing an LCEVC sample with split enhancement in different ways to test this: # decode video stream only. Enhancement stream is output as well and merged into video stream as side data, which lavc will see and use. # remux video stream only. Enhancement stream is output as well and merged into video stream as side data. Muxers will not care about it, but an hypothetical bsf could for example be inserted in the process to add it to the h264/hevc bitstream as a SEI message. # remux enhancement stream only. Video stream is output by lavf but discarded by the CLI. # remux both video and enhancement streams. Enhancement stream is merged into video stream as side data too.

+        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.

Ok.

OpenPGP_signature.asc
Description: OpenPGP digital signature

_______________________________________________
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".