This option flag only carries nontrivial information for options that
call a function, in all other cases its presence can be inferred from
the option type (bool options do not have arguments, all other types do)
and is thus nothing but useless clutter.
Change the option parsing code to infer its value when it can, and drop
the flag from options where it's not needed.
It causes those options to be parsed as either
* -autofoo 0/1 (with an argument)
* -noautofoo (without an argument)
This is unnecessary, confusing, and against the documentation; these are
also the only two bool options that take an argument.
This should not affect the users, as these options are on by default,
and are supposed to be used as -nofoo per the documentation.
Otherwise an unitialized stack value would be copied to FPSConvContext.
As it's then never used, it tends not to be a problem in practice,
however it is UB and some compilers warn about it.
Long is 32 bits signed on Windows, and nb_stream{s,_groups} are both unsigned
int. In a realistic scenario this wont make a difference, but it's still
proper.
Also ensure the parsed string is an integer while at it.
Signed-off-by: James Almer <jamrial@gmail.com>
In close_output(), a dummy frame is created with format NONE passed
to enc_open(), which isn't prepared for it. The NULL pointer
dereference happened at
av_pix_fmt_desc_get(enc_ctx->pix_fmt)->comp[0].depth.
When fgt.graph is NULL, skip fg_output_frame() since there is
nothing to output.
frame #0: 0x0000005555bc34a4 ffmpeg_g`enc_open(opaque=0xb400007efe2db690, frame=0xb400007efe2d9f70) at ffmpeg_enc.c:235:44
frame #1: 0x0000005555bef250 ffmpeg_g`enc_open(sch=0xb400007dde2d4090, enc=0xb400007e4e2daad0, frame=0xb400007efe2d9f70) at ffmpeg_sched.c:1462:11
frame #2: 0x0000005555bee094 ffmpeg_g`send_to_enc(sch=0xb400007dde2d4090, enc=0xb400007e4e2daad0, frame=0xb400007efe2d9f70) at ffmpeg_sched.c:1571:19
frame #3: 0x0000005555bee01c ffmpeg_g`sch_filter_send(sch=0xb400007dde2d4090, fg_idx=0, out_idx=0, frame=0xb400007efe2d9f70) at ffmpeg_sched.c:2154:12
frame #4: 0x0000005555bcf124 ffmpeg_g`close_output(ofp=0xb400007e4e2d85b0, fgt=0x0000007d1790eb08) at ffmpeg_filter.c:2225:15
frame #5: 0x0000005555bcb000 ffmpeg_g`fg_output_frame(ofp=0xb400007e4e2d85b0, fgt=0x0000007d1790eb08, frame=0x0000000000000000) at ffmpeg_filter.c:2317:16
frame #6: 0x0000005555bc7e48 ffmpeg_g`filter_thread(arg=0xb400007eae2ce7a0) at ffmpeg_filter.c:2836:15
frame #7: 0x0000005555bee568 ffmpeg_g`task_wrapper(arg=0xb400007d8e2db478) at ffmpeg_sched.c:2200:21
Signed-off-by: Zhao Zhili <zhilizhao@tencent.com>
They cannot store 1 as signed, only 0 and -1.
Avoids warnings such as:
implicit truncation from 'int' to a one-bit wide bit-field changes value from 1 to -1 [-Wsingle-bit-bitfield-constant-conversion]
Should fix valgrind warnings about Conditional jump or move depends on uninitialised value.
Reviewed-by: Anton Khirnov <anton@khirnov.net>
Signed-off-by: James Almer <jamrial@gmail.com>
Change the main loop and every component (demuxers, decoders, filters,
encoders, muxers) to use the previously added transcode scheduler. Every
instance of every such component was already running in a separate
thread, but now they can actually run in parallel.
Changes the results of ffmpeg-fix_sub_duration_heartbeat - tested by
JEEB to be more correct and deterministic.
See the comment block at the top of fftools/ffmpeg_sched.h for more
details on what this scheduler is for.
This commit adds the scheduling code itself, along with minimal
integration with the rest of the program:
* allocating and freeing the scheduler
* passing it throughout the call stack in order to register the
individual components (demuxers/decoders/filtergraphs/encoders/muxers)
with the scheduler
The scheduler is not actually used as of this commit, so it should not
result in any change in behavior. That will change in future commits.
As for the analogous decoding change, this is only a preparatory step to
a fully threaded architecture and does not yet make encoding truly
parallel. The main thread will currently submit a frame and wait until
it has been fully processed by the encoder before moving on. That will
change in future commits after filters are moved to threads and a
thread-aware scheduler is added.
This code suffers from a known issue - if an encoder with a sync queue
receives EOF it will terminate after processing everything it currently
has, even though the sync queue might still be triggered by other
threads. That will be fixed in following commits.
* the code is made shorter and simpler
* avoids constantly allocating and freeing AVPackets, thanks to
ThreadQueue integration with ObjPool
* is consistent with decoding/filtering/muxing
* reduces the diff in the future switch to thread-aware scheduling
This makes ifile_get_packet() always block. Any potential issues caused
by this will be resolved by the switch to thread-aware scheduling in
future commits.
Otherwise they'd be silently ignored if received by the filtering thread
before the filtergraph can be initialized, which would make the output
dependent on the order in which frames from different inputs arrive.
As previously for decoding, this is merely "scaffolding" for moving to a
fully threaded architecture and does not yet make filtering truly
parallel - the main thread will currently wait for the filtering thread
to finish its work before continuing. That will change in future commits
after encoders are also moved to threads and a thread-aware scheduler is
added.
Avoid making decisions based on current graph input state, which makes
the output dependent on the order in which the frames from different
inputs are interleaved.
Makes the output of fate-filter-overlay-dvdsub-2397 more correct - the
subtitle appears two frames later, which is closer to its PTS as stored
in the file.
Declaring the function argument as const fixes a warning down the line
that the const parameter is stripped. We don't modify this argument.
Signed-off-by: Leo Izen <leo.izen@gmail.com>
Signed-off-by: Zhao Zhili <zhilizhao@tencent.com>
Current code tracks min/max pts for each stream separately; then when
the file ends it combines them with last frame's duration to compute the
total duration of each stream; finally it selects the longest of those
durations as the file duration.
This is incorrect - the total file duration is the largest timestamp
difference between any frames, regardless of the stream.
Also change the way the last frame information is reported from decoders
to the muxer - previously it would be just the last frame's duration,
now the end timestamp is sent, which is simpler.
Changes the result of the fate-ffmpeg-streamloop-transcode-av test,
where the timestamps are shifted slightly forward. Note that the
matroska demuxer does not return the first audio packet after seeking
(due to buggy interaction betwen the generic code and the demuxer), so
there is a gap in audio.
This ensures that tq_receive() will always return EOF after all streams
were receive-finished, even though the sending side might not have
closed them yet. This may allow the receiver to avoid manually tracking
which streams it has already closed.
Fixes ticket #10638 (and should also fix ticket #10482)
by restoring the behaviour from before
3c7dd5ed37.
Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@outlook.com>
GetStdHandle is unavailable outside of the desktop API subset.
This didn't use to be a problem with earlier WinSDKs, as kbhit also
used to be available only for desktop apps, and this whole section is
wrapped in #if HAVE_KBHIT. With newer WinSDKs, kbhit() is available also
for non-desktop apps, while GetStdHandle still isn't.
Signed-off-by: Martin Storsjö <martin@martin.st>
Fix rendering of int values within a side data element, which was
broken since commit d2d3a83ad9, where the side data element was
correctly marked as a variable fields element. Logic to render a
string variable was implemented already, but it was not implemented
for the int fields path, which was enabled by that commit.
Also, code and schema is changed in order to account for multiple
variable-fields elements - such as side data, contained within the
same parent. Previously it was assumed that a single variable-fields
element was contained within the parent, which was the case for tags,
but is not the case for side-data.
Previously data was rendered as:
<side_data_list>
<side_data side_data_type="CPB properties" max_bitrate="0" min_bitrate="0" avg_bitrate="0" buffer_size="327680" vbv_delay="-1"/>
</side_data_list>
Now as:
<side_data_list>
<side_data type="CPB properties">
<side_datum key="side_data_type" value="CPB properties"/>
<side_datum key="max_bitrate" value="0"/>
<side_datum key="min_bitrate" value="0"/>
<side_datum key="avg_bitrate" value="0"/>
<side_datum key="buffer_size" value="49152"/>
<side_datum key="vbv_delay" value="-1"/>
</side_data>
</side_data_list>
Variable-fields elements are rendered as a containing element wrapping
generic key/values elements, enabling use of strict XML schema.
Fix trac issue:
https://trac.ffmpeg.org/ticket/10613
An AVFormatContext leaks on errors that happen before it is attached
to its permanent place (an InputFile). Fix this by attaching
it earlier.
Given that it is not documented that avformat_close_input() is usable
with an AVFormatContext that has only been allocated with
avformat_alloc_context() and not opened with avformat_open_input(),
one error path before avformat_open_input() had to be treated
specially: It uses avformat_free_context().
Reviewed-by: Anton Khirnov <anton@khirnov.net>
Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@outlook.com>
The av_opt_eval family of functions emits errors messages on error
and can therefore not be used with fake objects when the AVClass
has a custom item_name callback. The AVClass for AVCodecContext
has such a custom callback (it searches whether an AVCodec is set
to use its name). In practice it means that whatever is directly
after the "cc" pointer to the AVClass for AVCodec in the stack frame
of ist_add() will be treated as a pointer to an AVCodec with
unpredictable consequences.
Fix this by using an actual AVCodecContext instead of a fake object.
Reviewed-by: Anton Khirnov <anton@khirnov.net>
Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@outlook.com>
Its function is analogous to that of the fps filter, so filtering is a
more appropriate place for this.
The main practical reason for this move is that it places the encoding
sync queue right at the boundary between filters and encoders. This will
be important when switching to threaded scheduling, as the sync queue
involves multiple streams and will thus need to do nontrivial
inter-thread synchronization.
In addition to framerate conversion, the closely-related
* encoder timebase selection
* applying the start_time offset
are also moved to filtering.
That field is used by the framerate code to track whether any output has
been generated for the last input frame(*). Its use in the last
invocation of print_report() is meant to account for the very last
filtered frame being dropped in the number of dropped frames printed in
the log. However, that is a highly inappropriate place to do so, as it
makes assumptions about vsync logic in completely unrelated code. Move
the increment to encoder flush instead.
(*) the name is misleading, as the input frame has not yet been dropped
and may still be output in the future
Always use the functionality of the latter, which makes more sense as it
avoids losing keyframes due to vsync code dropping frames.
Deprecate the 'source_no_drop' value, as it is now redundant.
Unlike the 'source' mode, which preserves source keyframe-marking as-is,
the 'source_no_drop' mode attempts to keep track of keyframes dropped by
framerate conversion and mark the next output frame as key in such
cases. However,
* c75be06148 broke this functionality entirely, and made it equivalent
to 'source'
* even before it would only work when the frame immediately following
the dropped keyframe is preserved and not dropped as well
Also, drop a redundant check for 'frame' in setting dropped_keyframe, as
it is redundant with the check on the above line.
ifilter_send_eof() will fail if the input has no real or fallback
parameters, so there is no need to handle the case of some inputs being
in EOF state yet having no parameters.
This is no longer needed as the side data is available for decoders in the
AVCodecContext.
The tests affected reflect the removal of useless CPB and Stereo 3D side
data in packets.
Signed-off-by: James Almer <jamrial@gmail.com>
Also, avoid spurious end-of-line after side data entries, and improve
rendering of compact output, by adding an indication of the side data
type for each entry.
Also fixes issue:
http://trac.ffmpeg.org/ticket/9266
In this case any timestamps are guessed by compute_pkt_fields() in
libavformat. Since we are decoding the stream, we have more accurate
information from the decoder and do not need any guesses.
Eliminates spurious PTS gaps in a number of FATE tests.
Also avoids dropping the majority of frames in fate-dirac*
It is badly named (should have been -top_field_first, or at least -tff),
underdocumented and underspecified, and (most importantly) entirely
redundant with the setfield filter.
Merge three blocks with slightly inconsistent checks into one, treating
encoder input as interlaced when either:
* at least one of ilme/ildct flags is set
* the first frame in the stream is marked as interlaced
* the user specified the -top option
Stop modifying the frame passed to enc_open().
This is not an error condition, but would be treated like one if the
program terminates on the next transcode loop iteration because of a
signal or keyboard input.
Fixes#10504
Tested-by: https://github.com/0Ky
These defines are also used in other contexts than just AVCodecContext
ones, e.g. in libavformat. Furthermore, given that these defines are
public, the AV-prefix is the right one, so deprecate (and not just move)
the FF-macros.
Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@outlook.com>
The word "monotonous" means "spoken in a monotone" which is not what we
mean here. We mean "monotonic" i.e. nondecreasing.
Signed-off-by: Leo Izen <leo.izen@gmail.com>
When no output video framerate is specified by the user with -r or can
be inferred from the filtergraph, encoder setup will arbitrarily decide
that the framerate is 25fps. However, making up any framerate value for
VFR encoding is at best unnecessary.
Changes the results of the sub2video tests, where the input timebase is
now used instead of 1/25.
Mainly this fixes handling special values of -enc_time_base ('demux' or
'filter') for audio. It also prints a warning if -enc_time_base is
specified for subtitles, instead of ignoring it silently (current
subtitle encoding API only works with AV_TIME_BASE_Q).
This function converts packet timestamps from the input stream timebase
to OutputStream.mux_timebase, which may or may not be equal to the
actual output AVStream timebase (and even when it is, this may not
always be the optimal choice due to bitstream filtering).
Just keep the timestamps in input stream timebase, they will be rescaled
as needed before bitstream filtering and/or sending the packet to the
muxer.
Move the av_rescale_delta() call for audio (needed to preserve accuracy
with coarse demuxer timebases) to write_packet.
Drop now-unused OutputStream.mux_timebase.
Bitstream filtering input timebase is not always necessarily equal to
OutputStream.mux_timebase. Also, set AVPacket.time_base correctly for
packets output by bitstream filters
Do not rescale at all in of_output_packet() when not doing bitstream
filtering, as it's unnecessary - write_packet() will rescale to the
actual muxer timebase.
This is a bit cleaner as int need not be the underlying type
of an enum if a smaller type can hold all its values.
Also declare the children_ids array as const as it never changes.
Reviewed-by: Stefano Sabatini <stefasab@gmail.com>
Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@outlook.com>
Fixes Coverity issue #1524491.
Regression since e6126abc69.
Reviewed-by: James Almer <jamrial@gmail.com>
Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@outlook.com>
ffmpeg CLI pixel format selection for filtering currently special-cases
MJPEG encoding, where it will restrict the supported list of pixel
formats depending on the value of the -strict option. In order to get
that value it will apply it from the options dict into the encoder
context, which is a highly invasive action even now, and would become a
race once encoding is moved to its own thread.
The ugliness of this code can be much reduced by moving the special
handling of MJPEG into ofilter_bind_ost(), which is called from encoder
init and is thus synchronized with it. There is also no need to write
anything to the encoder context, we can evaluate the option into our
stack variable.
There is also no need to access AVCodec at all during pixel format
selection, as the pixel formats array is already stored in
OutputFilterPriv.
When -pix_fmt designates a BE/LE pixel format, it gets translated into
the native one by av_get_pix_fmt(). This may not always be the best
choice, as the encoder might only support one endianness. In such a
case, explicitly choose the endianness supported by the encoder.
While this is currently redundant with choose_pixel_fmt() in
ffmpeg_filter.c, the latter code will be deprecated in following commits.
frame is always != NULL for audio and video here
(this is checked by an assert and the frame is already dereferenced
before it reaches this code here).
Fixes Coverity issue #1538858.
Reviewed-by: Anton Khirnov <anton@khirnov.net>
Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@outlook.com>
map_func is supposed to be an array of const pointer to function
returning int, not an array of pointer to function returning const int.
Reported-By: Martin Storsjö
Read the timebase from FrameData rather than the input stream. This
should fix#10393 and generally be more reliable.
Replace the use of '-1' to indicate demuxing timebase with the string
'demux'. Also allow to request filter timebase with
'-enc_time_base filter'.
It now contains data from multiple sources, so group those items that
always come from the decoder. Also, initialize them to invalid values,
so that frames that did not originate from a decoder can be
distinguished.
This is possible now that enc_open() is always called with a non-NULL
frame for audio/video.
Previously the code would directly reach into the buffersink, which is a
layering violation.
When no frames were passed from a filtergraph to an encoder, but the
filtergraph is configured (i.e. has output parameters), encoder flush
code will use those parameters to initialize the encoder in a last-ditch
effort to produce some useful output.
Rework this process so that it is triggered by the filtergraph, which
now sends a dummy frame with parameters, but no data, to the encoder,
rather than the encoder reaching backwards into the filter.
This approach is more in line with the natural data flow from filters to
encoders and will allow to reduce encoder-filter interactions in
following commits.
This code is tested by fate-adpcm-ima-cunning-trunc-t2-track1, which (as
confirmed by Zane) is supposed to produce empty output.
This line was added in c30a4489b4 along with
AVStream.sample_aspect_ratio. However, configuring SAR for video
encoding is now done after this code (specifically in enc_open(), which
is called when the first video frame to be encoded is obtained), so this
line cannot have any meaningful effect.
Replace duplicated(!) and broken* custom string parsing with
av_dict_parse_string(). Return error codes instead of aborting.
* e.g. it treats NULL returned from av_get_token() as "separator not
found", when in fact av_get_token() only returns NULL on memory
allocation failure
EOF from sq_receive() means no packets will ever be output by the sync
queue. Since the muxing sync queue is always used by all interleaved
(i.e. non-attachment) streams, this means no further packets can make
it to the muxer and we can terminate muxing now.
Also fix a couple of possible overflows while at it.
Fixes the negative initial timestamps in ticket #10358.
Signed-off-by: Marton Balint <cus@passwd.hu>
Not sure if the function naming frame_queue_destory is intended because
"destory" is not really a word. Changing it to "destroy" makes more sense.
Signed-off-by: QiTong Li <liqitong@163.com>
Signed-off-by: Marton Balint <cus@passwd.hu>
This is only a preparatory step to a fully threaded architecture and
does not yet make decoding truly parallel - the main thread will
currently submit a packet and wait until it has been fully processed by
the decoding thread before moving on. Decoder behavior as observed by
the rest of the program should remain unchanged. That will change in
future commits after encoders and filters are moved to threads and a
thread-aware scheduler is added.
It is only used for flushing the subtitle decoder, so allocate a
dedicated packet for that.
Keep Decoder.pkt unused for now, it will be repurposed in future
commits.
Make the function process just one input stream at a time and save an
indentation level. Also rename it to ist_add() to be consistent with an
analogous function in ffmpeg_mux_init.
Currently of_output_packet() reuses the input packet, which requires its
callers to submit blank packets even on EOF, which makes the code more
complex.
It is set by the muxing code, which will not be synchronized with
encoding code after upcoming threading changes. Use an encoder-private
variable instead.
Packets submitted to the muxer now have their timebase attached to them,
so the muxer can do conversion to muxing timebase and avoid exposing it
to callers.
There is no reason to postpone it until opening the encoder. Also, abort
when the input stream is unknown, rather than disregard an explicit
request from the user.
The code will currently add a small offset to avoid exact midpoints, but
this can cause inexact results when a float timestamp is exactly
representable as an integer.
Fixes off-by-one in the first frame duration in multiple FATE tests.
Anton Khirnov
James Almer
Zhao Zhili
Leo Izen
Andreas Rheinhardt
Martin Storsjö
Stefano Sabatini
Lynne
Marton Balint
QiTong Li