This causes regressions in end to end timestamps with mp3s and ffmpeg.
The revert is to avoid this regression in the 4.3 release
See: [FFmpeg-devel] [PATCH] Don't adjust start time for MP3 files; packets are not adjusted.
This reverts commit 460132c998.
7546ac2fee made it so that the start_time for mp3 files is
adjusted for skip_samples. However, this appears incorrect because
subsequent packet timestamps are not adjusted and skip_samples are
applied by deleting data from a packet without changing the timestamp.
E.g., we are told the start_time is ~25ms and we get a packet with a
timestamp of 0 that has had the skip_samples discarded from it. As such
rendering engines may incorrectly discard everything prior to the
25ms thinking that is where playback should officially start. Since the
samples were deleted without adjusting timestamps though, the true
start_time is still 0.
Other formats like MP4 with edit lists will adjust both the start
time and the timestamps of subsequent packets to avoid this issue.
Signed-off-by: Dale Curtis <dalecurtis@chromium.org>
Signed-off-by: Anton Khirnov <anton@khirnov.net>
mp3 header bitstream syntax:
header()
{
syncword 12bits bslsf
id 1bit bslsf
layer 2bit bslsf
protection_bit 1bit bslsf
bitrate_index 4bits bslsf
sampling_frequency 2bits bslsf
padding_bit 1bit bslsf
private_bit 1bit bslsf
mode 2bits bslsf
mode_extension 2bits bslsf
copyright 1bit bslsf
original/home 1bit bslsf
emphasis 2bits bslsf
}
if the header is masking with MP3_MASK(0xFFFE0CCF), below fields will be cleared:
protection_bit, bitrate_index, sampling_freqency, mode
with SAME_HEADER_MASK(0xFFFE0C00), extra below fields will be cleared which didn't make
sense:
mode_extension, copyright, original/home, emphasis
As the MP3_MASK is good for same mp3 header masking and is defined in the
header, so it's preferable to remove SAME_HEADER_MASK to keep the masking same.
Signed-off-by: Limin Wang <lance.lmwang@gmail.com>
Signed-off-by: Michael Niedermayer <michael@niedermayer.cc>
This massively reduces the detection of random data as low score mp3
It may improve security by making it harder to read non multimedia data
Signed-off-by: Michael Niedermayer <michael@niedermayer.cc>
The number of bits from bit #m to #n is n - m plus 1.
Signed-off-by: Ingo Brückl <ib@wupperonline.de>
Signed-off-by: Michael Niedermayer <michael@niedermayer.cc>
MPEG Audio frame header must be 4 bytes. If we fail to read
4 bytes bail early to avoid Use-of-uninitialized-value msan error.
Reference https://crbug.com/666874.
Signed-off-by: Michael Niedermayer <michael@niedermayer.cc>
"Skipping 0 bytes of junk" is useless to the user, and essentially
indicates a NOP. At 0 bytes, this message is now pushed back to
the verbose log level.
Signed-off-by: Moritz Barsnick <barsnick@gmx.net>
Currently, AVStream contains an embedded AVCodecContext instance, which
is used by demuxers to export stream parameters to the caller and by
muxers to receive stream parameters from the caller. It is also used
internally as the codec context that is passed to parsers.
In addition, it is also widely used by the callers as the decoding (when
demuxer) or encoding (when muxing) context, though this has been
officially discouraged since Libav 11.
There are multiple important problems with this approach:
- the fields in AVCodecContext are in general one of
* stream parameters
* codec options
* codec state
However, it's not clear which ones are which. It is consequently
unclear which fields are a demuxer allowed to set or a muxer allowed to
read. This leads to erratic behaviour depending on whether decoding or
encoding is being performed or not (and whether it uses the AVStream
embedded codec context).
- various synchronization issues arising from the fact that the same
context is used by several different APIs (muxers/demuxers,
parsers, bitstream filters and encoders/decoders) simultaneously, with
there being no clear rules for who can modify what and the different
processes being typically delayed with respect to each other.
- avformat_find_stream_info() making it necessary to support opening
and closing a single codec context multiple times, thus
complicating the semantics of freeing various allocated objects in the
codec context.
Those problems are resolved by replacing the AVStream embedded codec
context with a newly added AVCodecParameters instance, which stores only
the stream parameters exported by the demuxers or read by the muxers.
Almost all the places from which this function is called already check
the header manually and in the two that don't (the mp3 muxer) the check
should not cause any problems.
"Fast seek" uses linear interpolation to find the position of the
requested seek time. For CBR this is more direct than using the
mp3 TOC and bypassing the TOC avoids problems with TOC precision.
(see https://crbug.com/545914#c13)
For VBR, fast seek is not precise, so continue to prefer the TOC
when available (the lesser of two evils).
Also, some re-ordering of the logic in mp3_seek to simplify and
give usetoc=1 precedence over fastseek flag.
Signed-off-by: wm4 <nfxjfg@googlemail.com>
Commit 2b3e9bbfb5 caused problems for a
certain API user:
https://code.google.com/p/chromium/issues/detail?id=537725https://code.google.com/p/chromium/issues/detail?id=542032
The problem seems rather arbitrary, because if there's junk, anything
can happen. In this case, the imperfect junk skipping just caused it to
read different junk, from what I can see.
We can improve the accuracy of junk detection by a lot by checking if 2
consecutive frames use the same configuration. While in theory it might
be completely fine for the 1st frame to have a different format than the
2nd frame, it's exceedingly unlikely, and I can't think of a legitimate
use-case.
This is approximately the same mpg123 does for junk skipping. The
set of compared header bits is the same as the libavcodec mp3 parser
uses for similar purposes.
When AVFMT_FLAG_FAST_SEEK is specified, make MP3 seek operation as
fast as possible.
When no "-usetoc" is specified, the default operation is using TOC
if available; otherwise, uses linear interpolation. This is useful
when seeking a large MP3 file with no TOC available. One example is
Podcast, many MP3 files are large, but no CBR/VBR tags. Most of
them are actually CBR. Even in VBR cases, this option sacrifices the
accuracy of playback time in exchange for responsiveness.
The code is simply broken, the read packets are not aligned to
the mp3 frames, the file end or the id3 tag thus this simply
cannot reliably find the ID3v1 tag to remove it
Signed-off-by: Michael Niedermayer <michaelni@gmx.at>
Apparently it can happen that a mp3 file has junk data between id3 tag
and actual mp3 data. Skip this to avoid outputting nonsense timestamps.
(Two packets had the same timestamps, because the mp3 parser failed to
compute a frame duration.)
In this case, the junk consisted of 1044 bytes of zero, which
incidentally is the same size as normal mp3 frames in this stream. I
suspect the mp3 was edited with some tool which wiped the Xing/LAME
headers. Data near the end of the file suggests it was encoded with
"LAME3.97", but the normal Xing/LAME headers are missing. So this could
be "normal". mpg123 also attempts to skip at least 64KB of junk data by
scanning for headers.
Signed-off-by: Michael Niedermayer <michaelni@gmx.at>
If the file size is much larger than what is indicated in the XING
header, the demuxer assumes it's a concatenated file, and throws away
the (presumably) incorrect duration information. Unfortunately, this
also triggers if the id3 tags are very large (embedded pictures and
such). Then the half-baked heuristic not only breaks the duration
display, but also gapless audio.
Fix it by subtracting the size of the headers (the check is off by some
bytes, but that doesn't matter at all). Note that there could be an
arbitrary amount of tags _after_ the mp3 data, but hopefully these are
not too large to trigger the heuristic in practice.
Also add a warning when this happens.
Signed-off-by: Michael Niedermayer <michaelni@gmx.at>
It's the most useful mode, because it seeks accurately, and does not
break features like gapless audio.
Signed-off-by: Michael Niedermayer <michaelni@gmx.at>
"-usetoc 2" now invokes the generic seek and indexing mode. This mode
skips data until the seek target is reached, and this is exact. It also
makes gapless audio actually work if a seek past the start of the file
is involved.
Change the fate-gapless-mp3 test to use the new mode, and move the old
one to fate-gapless-mp3-toc (since the test forces use of the Xing TOC).
The new mode has a different result for the seek - this result is
actually correct.
Signed-off-by: Michael Niedermayer <michaelni@gmx.at>
The mp2 seek test results change. Whether to skip samples if the file
had no LAME gapless tags was inconsistent. When seeking to the start
of the file, 529 samples were skipped, but when playing from start,
nothing was skipped. This commit changes the behavior on seek to skip
nothing.
Signed-off-by: Michael Niedermayer <michaelni@gmx.at>
With this the returned timestamp should match the packet instead of
the requested timestamp, which may lay between packets
Reviewed-by: wm4 <nfxjfg@googlemail.com>
Signed-off-by: Michael Niedermayer <michaelni@gmx.at>
Removing a bunch of questionable hacks makes it work. These hacks
apparently try to make concatenated mp3s with Lame headers seekable,
which doesn't make too much sense anyway. The main change is that we
trust the Xing header file size field now (the same field is used for
seeking with Xing TOC). Note that a mp3 might contain an unknown number
of unsupported additional tags, so we can't reliably compute this size
manually.
Signed-off-by: Michael Niedermayer <michaelni@gmx.at>
Michael Niedermayer
Dale Curtis
Limin Wang
Carl Eugen Hoyos
Steven Liu
Ingo Brückl
Paul Arzelier
Chris Cunningham
Anton Khirnov
Diego Biurrun
Moritz Barsnick
Derek Buitenhuis
Clément Bœsch
wm4
Andy Wu
James Almer
Ganesh Ajjanagadde
Luca Barbato
Michael Niedermayer
Carl Eugen Hoyos
Vittorio Giovara