The two checks using eof_reached are testing whether more input can
possibly appear on this filtergraph input. InputFilterPriv.eof is the
more authoritative source for this information.
When an input stream terminates and no frames were successfully decoded,
filtering code will currently configure the filtergraph using demuxer
stream parameters. Use decoder parameters instead, which should be more
reliable. Also, initialize them immediately when an input stream is
bound to a filtergraph input, so that these parameters are always
available (if at all) and filtering code does not need to reach into the
decoder at some arbitrary later point.
When no frames are ever seen by an encoder, encoder flush will do a
last-ditch attempt to configure its source filtergraph in order to at
least get the stream parameters. This involves extracting demuxer
parameters from filtergraph source inputs, which is
* a bad layering violation
* probably unreachable, because decoders are flushed before encoders,
which should call ifilter_send_eof(), which will also set these
parameters; however due to complex control flow it is hard to be
entirely sure this code can never be triggered
Even if this code can actually be reached, it is probably better to
return an error as the comment above it says.
These two functions are a part of a single logical action - determining
which, if any, output stream needs to be processed next. Keeping them
separate is a historical artifact that obscures what is actually being
done.
Currently those are set in different ways depending on whether the
stream is decoded or not, using some values from the decoder if it is.
This is wrong, because there may be arbitrary amount of delay between
input packets and output frames (depending e.g. on the thread count when
frame threading is used).
Always use the path that was previously used only for streamcopy. This
should not cause any issues, because these values are now used only for
streamcopy and discontinuity handling.
This change will allow to decouple discontinuity processing from
decoding and move it to ffmpeg_demux. It also makes the code simpler.
Changes output in fate-cover-art-aiff-id3v2-remux and
fate-cover-art-mp3-id3v2-remux, where attached pictures are now written
in the correct order. This happens because InputStream.dts is no longer
reset to AV_NOPTS_VALUE after decoding, so streamcopy actually sees
valid dts values.
This was added in 380db56928 as a
temporary crutch that is not needed anymore. The only case where this
code can be triggered is the very first frame, for which InputStream.pts
is always equal to 0.
Stop using InputStream.dts for generating missing timestamps for decoded
frames, because it contains pre-decoding timestamps and there may be
arbitrary amount of delay between input packets and output frames (e.g.
dependent on the thread count when frame threading is used). It is also
in AV_TIME_BASE (i.e. microseconds), which may introduce unnecessary
rounding issues.
New code maintains a timebase that is the inverse of the LCM of all the
samplerates seen so far, and thus can accurately represent every audio
sample. This timebase is used to generate missing timestamps after
decoding.
Changes the result of the following FATE tests
* pcm_dvd-16-5.1-96000
* lavf-smjpeg
* adpcm-ima-smjpeg
In all of these the timestamps now better correspond to actual frame
durations.
If input packets have timestamps, they will be propagated to output
frames by the decoder, so at best this block does not do anything.
There can also be an arbitrary amount of delay between packets sent to
the decoder and decoded frames (e.g. due to decoder's intrinsic delay or
frame threading), so deriving any timestamps from packet properties is
wrong.
The previous name is misleading, because the function does not actually
initialize any filters - it creates a new output stream and binds a
filtergraph output to it.
Their only function is checking that encoding was not specified for
data/unknown-type streams, but the check is broken because enc_ctx will
not be created in ost_add() unless a valid encoder can be found.
Add an actually working check for all types for which encoding is not
supported in choose_encoder().
ts_discontinuity_detect() is applied right after demuxing, while
InputStream.pts is a post-decoding timestamp, which may be delayed with
respect to demuxing by an arbitrary amount (e.g. depending on the thread
count when frame threading is used).
The name is misleading, because it is not a picture in the sense of MPEG
terminology (which define "picture" as "frame or field"), but always a
full frame. 'next' is also redundant and/or misleading, because it is
the _current_ frame to be encoded.
Previously they would only be used with trivial filtergraphs, because
filters did not handle frame durations. That is no longer true - most
filters process frame durations properly (there may still be some that
don't - this change will help finding and fixing them).
Improves output video frame durations in a number of FATE tests.
When an encoder exports sum-of-squared-differences information in
encoded packets, print_report() will print PSNR information in the
status line. However,
* the code computing PSNR assumes 8bit 420 video and prints incorrect
values otherwise; there are no issues on trac about this
* only a few encoders (namely aom, vpx, mpegvideo, snow) export this
information; other often-used encoders such as libx26[45] do not
export this, even though they could
This suggests that this feature is not useful and it is better to remove
it rather than spend effort on fixing it.
Remove now-obsolete code setting packet durations pre-muxing for CFR
encoded video.
Changes output in the following FATE tests:
* numerous adpcm tests
* ffmpeg-filter_complex_audio
* lavf-asf
* lavf-mkv
* lavf-mkv_attachment
* matroska-encoding-delay
All of these change due to the fact that the output duration is now
the actual input data duration and does not include padding added by
the encoder.
* apng-osample: less wrong packet durations are now passed to the muxer.
They are not entirely correct, because the first frame duration should
be 3 rather than 2. This is caused by the vsync code and should be
addressed later, but this change is a step in the right direction.
* tscc2-mov: last output frame has a duration of 11 rather than 1 - this
corresponds to the duration actually returned by the demuxer.
* film-cvid: video frame durations are now 2 rather than 1 - this
corresponds to durations actually returned by the demuxer and matches
the timestamps.
* mpeg2-ticket6677: durations of some video frames are now 2 rather than
1 - this matches the timestamps.
That field was added to store timestamp conversion state for audio
decoding code. Later it started being used by streamcopy as well, but
that use is wrong because, for a given input stream, both decoding and
an arbitrary number of streamcopies may be performed simultaneously.
They would then all overwrite the same state variable.
Store this state in MuxStream instead.
This is the last use of InputStream in of_streamcopy(), so the ist
parameter can now be removed.
It stores codec parameters of the stream submitted to the muxer, which
may be different from the codec parameters in AVStream due to bitstream
filtering.
This avoids the confusing back and forth synchronisation between the
encoder, bitstream filters, and the muxer, now information flows only in
one direction. It also reduces the need for non-muxing code to access
AVStream.
Reduces access to a deeply nested muxer property
OutputStream.st->codecpar->codec_type for this fundamental and immutable
stream property.
Besides making the code shorter, this will allow making the AVStream
(OutputStream.st) private to the muxer in the future.
Set InputStream.decoding_needed/discard/etc. only from
ist_{filter,output},add() functions. Reduces the knowledge of
InputStream internals in muxing/filtering code.
init_input_stream() can print log messages directly, there is no need to
ship them to the caller.
Also, log errors to the InputStream and avoid duplicate information in
the message.
Changing AVCodecContext.sample_aspect_ratio after the encoder was opened
is by itself questionable, but if anywhere it belongs in encoding rather
than filtering code.
Creating a new output stream of a given type is currently done by
calling new_<type>_stream(), which all start by calling
new_output_stream() to allocate the stream and do common init, followed
by type-specific init.
Reverse this structure - the caller now calls the common function
ost_add() with the type as a parameter, which then calls the
type-specific function internally. This will allow adding common code
that runs after type-specific code in future commits.
In most cases this should only occur once or so per stream in an
input, and in case the logic ends up in an eternal loop, it should
be visible to the end user instead of being completely invisible.
Signed-off-by: Jan Ekström <jan.ekstrom@24i.com>
When no packet dts values are available from the container, video
decoding code will currently use its own guessed values, which will then
be propagated to pkt_dts on decoded frames and used as pts in certain
cases. This is inaccurate, fragile, and unnecessarily convoluted.
Simply removing this allows the extrapolation code introduced in the
previous commit to do a better job.
Changes the results of numerous h264 and hevc FATE tests, where no
spurious timestamp gaps are generated anymore. Several tests no longer
need -vsync passthrough.
When no timestamps are available from the container, the video decoding
code will currently use fake dts values - generated in
process_input_packet() based on a combination of information from the
decoder and the parser (obtained via the demuxer) - to generate
timestamps during decoder flushing. This is fragile, hard to follow, and
unnecessarily convoluted, since more reliable information can be
obtained directly from post-decoding values.
The new code keeps track of the last decoded frame pts and estimates its
duration based on a number of heuristics. Timestamps generated when both
pts and pkt_dts are missing are then simple pts+duration of the last frame.
The heuristics are somewhat complicated by the fact that lavf insists on
making up packet timestamps based on its highly incomplete information.
That should be removed in the future, allowing to further simplify this
code.
The results of the following tests change:
* h264-3386 now requires -fps_mode passthrough to avoid dropping frames
at the end; this is a pathology of the interaction of the new and old
code, and the fact that the sample switches from field to frame coding
in the last packet, and will be fixed in following commits
* hevc-conformance-DELTAQP_A_BRCM_4 stops inventing an arbitrary
timestamp gap at the end
* hevc-small422chroma - the single frame output by this test now has a
timestamp of 0, rather than an arbitrary 7
This field contains different values depending on whether the stream is
being decoded or not. When it is, InputStream.pts is set to the
timestamp of the last decoded frame. Otherwise, it is made equal to
InputStream.dts.
Since a given InputStream can be at the same time decoded and
streamcopied to any number of output streams, this use is incorrect, as
decoded frame timestamps can be delayed with respect to input packets by
an arbitrary amount (e.g. depending on the thread count when frame
threading is used).
Replace all uses of InputStream.pts for streamcopy with InputStream.dts,
which is its value when decoding is not performed. Stop setting
InputStream.pts for pure streamcopy.
Also, pass InputStream.dts as a parameter to do_streamcopy(), which
will allow that function to be decoupled from InputStream completely in
the future.
Which is subtitle encoding. Also, check for AVSubtitle.pts rather than
InputStream.pts, since that is the more authoritative value and is
guaranteed to be valid.
That function only contains two checks now - whether the muxer returned
EOF and whether the packet timestamp is before requested output start
time.
The first check is unnecessary, since the packet will just be rejected
by the muxer. The second check is better combined with a related check
directly in do_streamcopy().
Currently, output streams where an input stream is sent directly (i.e.
not through lavfi) are determined by iterating over ALL the output
streams and skipping the irrelevant ones. This is awkward and
inefficient.
The channel layout is set before opening the encoder, in enc_open().
Messing with it in configure_output_audio_filter() cannot accomplish
anything meaningful.
This option adds a long string of numbers to the progress line, where
i-th number contains the base-2 logarithm of the number of times a frame
with this QP value was seen by print_report().
There are multiple problems with this feature:
* despite this existing since 2005, web search shows no indication
that it was ever useful for any meaningful purpose;
* the format of what is printed is entirely undocumented, one has to
find it out from the source code;
* QP values above 31 are silently ignored;
* it only works with one video stream;
* as it relies on global state, it is in conflict with ongoing
architectural changes.
It then seems that the nontrivial cost of maintaining this option is not
worth its negligible (or possibly negative - since it pollutes the
already large option space) value.
Users who really need similar functionality can also implement it
themselves using -vstats.
Current code in print_final_stats(), printing the final summary such as
video:8851kB audio:548kB subtitle:0kB other streams:0kB global headers:20kB muxing overhead: 0.559521%
was written with a single output file in mind and makes very little
sense otherwise.
Print this information in mux_final_stats() instead, one line per output
file. Use the correct filesize, if available.
This is currently done in two places:
* at the end of print_final_stats(), which merely prints a warning if
the total size of all written packets is zero
* at the end of transcode() (under a misleading historical 'close each
encoder' comment), which instead checks the packet count to implement
-abort_on empty_output[_stream]
Consolidate both of these blocks into a single function called from
of_write_trailer(), which is a more appropriate place for this. Also,
return an error code rather than exit immediately, which ensures all
output files are properly closed.
Properly pass muxing return codes through the call stack instead.
Slightly changes behavior in case of errors:
* the output IO stream is closed even if writing the trailer returns an
error, which should be more correct
* all files get properly closed with -xerror, even if one of them fails
It is video encoding-only and does not need to be visible outside of
ffmpeg_enc.c
Also, rename the variable to frames_prev_hist to be consistent with
the naming in do_video_out().
Drop unneeded ctype.h and math.h.
Group all system headers together.
Sort unconditional includes alphabetically.
Group local includes by the library, sort alphabetically.
Several places in the code currently call init_output_stream_wrapper(),
which in turn calls init_output_stream(), which then calls either
enc_open() or init_output_stream_streamcopy(), followed by
of_stream_init(), which tells the muxer the stream is ready for muxing.
All except one of these callers are in the encoding code, which will be
moved to ffmpeg_enc.c. Keeping this structure would then necessitate
ffmpeg_enc.c calling back into the common code in ffmpeg.c, which would
then just call ffmpeg_mux, thus making the already convoluted call chain
even more so.
Simplify the situation by using separate paths for filter-fed output
streams (audio and video encoders) and others (subtitles, streamcopy,
data, attachments).
Encoder initialization is currently split rather arbitrarily between
init_output_stream_encode() and init_output_stream(). Move all of it to
init_output_stream_encode().
The code currently uses lavfi for this, which creates a sort of
configuration dependency loop - the encoder should be ideally
initialized with information from the first audio frame, but to get this
frame one needs to first open the encoder to know the frame size. This
necessitates an awkward workaround, which causes audio handling to be
different from video.
With this change, audio encoder initialization is congruent with video.
For audio AVFrames, nb_samples is typically more trustworthy than
duration. Since sync queues look at durations, make sure they match the
sample count.
The last audio frame in the fate-shortest test is now gone. This is more
correct, since it outlasts the last video frame.
