[FFmpeg-devel] [PATCH 24/35] fftools/ffmpeg: use the sync queues to handle -frames

[Anton Khirnov]

Quoting Andreas Rheinhardt (2022-06-22 10:27:30)
> Anton Khirnov:
> > Quoting Andreas Rheinhardt (2022-06-16 22:33:46)
> >> Anton Khirnov:
> >>> Same issues apply to it as to -shortest.
> >>>
> >>> Changes the results of the following tests:
> >>> - matroska-flac-extradata-update
> >>>   The test reencodes two input FLAC streams into three output FLAC
> >>>   streams. The last output stream is limited to 8 frames. The current
> >>>   code results in the first two output streams having 12 frames, after
> >>>   this commit all three streams have 8 frames and are the same length.
> >>>   This new result is better, since it is predictable.
> >>
> >> The point of the test was that only one stream is limited so that one
> >> can see the extradata update directly in the test result: The unlimited
> >> streams have a different extradata than the limited stream (because said
> >> extradata contains an md5 of the decoded data). So it is expected that
> >> the extradata hashes of the first two streams coincide and differ from
> >> the hash of the last stream.
> > 
> > Right, but my point is that the amount of data that ends up in those
> > unlimited streams is largely an accident of how the code happens to
> > work currently and is not guaranteed by anything.
> > 
> 
> The documentation of frames reads:
> "-frames[:stream_specifier] framecount (output,per-stream)
>     Stop writing to the stream after framecount frames."
> It does not say that the other streams end after one stream has reached
> its framecount. So it is guaranteed by the documentation that the other
> streams don't end prematurely.
> (Why do you think that this is an accident?)

1) Documentation not saying what happens to the other streams does not imply
   any guarantees IMO, it implies a lack of any guarantees.
2) Documentation for ffmpeg has always been less than fully descriptive
   (to put it extremely mildly), so I would not rely on it as the
   ultimate source of truth.
3) The original commit adding this option in 2004 would terminate ALL
   output to the file on reaching the specified frame limit, not just
   the affected stream. This was broken by me in
   2f51ec2b9438e211f5b8abb2fcf5d8be678e7e8c, because need_output()
   terminates early and does not check max_frames for further streams if
   an earlier stream still allows output. I would consider this a bug,
   because it makes no sense to treat the option differently based on
   stream ordering. Presumably nobody noticed because nobody relies on
   this option producing consistent output with multiple streams.
   This is also the reason the test outputs all frames for the first two
   streams. If you changed "-frames:a:2 8" to "-frames:a:0 8", you would
   get:
   Output stream #0:0 (audio): 8 frames encoded (36864 samples); 8 packets muxed (208 bytes);
   Output stream #0:1 (audio): 7 frames encoded (32256 samples); 8 packets muxed (182 bytes);
   Output stream #0:2 (audio): 8 frames encoded (36864 samples); 9 packets muxed (208 bytes);

-- 
Anton Khirnov