Re: [FFmpeg-devel] [PATCH] avformat/oggparseogm: sync avctx w/ codecpar

Friendly ping. On Thu, Jun 20, 2019 at 11:17 AM Chris Cunningham wrote: > On Thu, Feb 28, 2019 at 9:13 AM James Almer wrote: > >> On 2/26/2019 10:18 PM, Chris Cunningham wrote: >> > On Thu, Feb 21, 2019 at 4:46 PM Chris Cunningham >> > mailto:chc

Re: [FFmpeg-devel] [PATCH] avformat/oggparseogm: sync avctx w/ codecpar

On Thu, Feb 28, 2019 at 9:13 AM James Almer wrote: > On 2/26/2019 10:18 PM, Chris Cunningham wrote: > > On Thu, Feb 21, 2019 at 4:46 PM Chris Cunningham > > mailto:chcunning...@chromium.org>> wrote: > > > > I'm fine to do either. James, do you still pr

Re: [FFmpeg-devel] [PATCH] avformat/oggdec: only parse headers before data

On Wed, Jun 19, 2019 at 7:11 PM Chris Cunningham wrote: > On Wed, Jun 19, 2019 at 11:25 AM Michael Niedermayer > wrote: > >> breaks: >> ./ffmpeg -i bgc.sub.dub.ogm -vframes 3 -y test.webm >> sample: http://samples.mplayerhq.hu/ogg/bgc.sub.dub.ogm >> >&

Re: [FFmpeg-devel] [PATCH] avformat/oggdec: only parse headers before data

On Wed, Jun 19, 2019 at 11:25 AM Michael Niedermayer wrote: > breaks: > ./ffmpeg -i bgc.sub.dub.ogm -vframes 3 -y test.webm > sample: http://samples.mplayerhq.hu/ogg/bgc.sub.dub.ogm > > [...] > > -- > Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB > Thanks Michael. That

Re: [FFmpeg-devel] [PATCH] avformat/oggdec: only parse headers before data

+James On Mon, Jun 17, 2019 at 6:31 PM Chris Cunningham wrote: > This behavior was added in 2010 to suport some old (and invalid) ogm > files. > https://github.com/FFmpeg/FFmpeg/commit/81b743eb1026547270b88ac6a5cb451a3907ee94 > > But this makes it possible to change the cod

Re: [FFmpeg-devel] [PATCH] avformat/oggparseogm: unknown codec triggers error

mes Almer wrote: > > > On 6/14/2019 11:52 AM, Reimar Döffinger wrote: > >> > >> > >> On 14.06.2019, at 03:15, Chris Cunningham > wrote: > >> > >>> Only "succeed" to read a header if the codec is valid. Otherwise > >>

[FFmpeg-devel] [PATCH] avformat/oggdec: only parse headers before data

This behavior was added in 2010 to suport some old (and invalid) ogm files. https://github.com/FFmpeg/FFmpeg/commit/81b743eb1026547270b88ac6a5cb451a3907ee94 But this makes it possible to change the codec in the later headers, causing codec to be out of sync with internal avctx (eventually trigger

Re: [FFmpeg-devel] [PATCH] avformat/oggparseogm: unknown codec triggers error

+James This is patch is a follow up from an earlier thread: https://patchwork.ffmpeg.org/patch/11983/ I've implemented the easy part of that proposal. We also discussed disallowing multiple headers in an ogm change (generally a revert of: https://github.com/FFmpeg/FFmpeg/commit/81b743eb102654727

[FFmpeg-devel] [PATCH] avformat/oggparseogm: unknown codec triggers error

Only "succeed" to read a header if the codec is valid. Otherwise return AVERROR_INVALIDDATA. --- libavformat/oggparseogm.c | 12 ++-- 1 file changed, 10 insertions(+), 2 deletions(-) diff --git a/libavformat/oggparseogm.c b/libavformat/oggparseogm.c index a07453760b..e71298d39a 100644 ---

Re: [FFmpeg-devel] [PATCH] avformat/oggparseogm: sync avctx w/ codecpar

On Thu, Feb 21, 2019 at 4:46 PM Chris Cunningham wrote: > I'm fine to do either. James, do you still prefer to skip the later > headers if this breaks some old ogm files? > James, friendly ping ___ ffmpeg-devel mailing list ffmpeg-de

Re: [FFmpeg-devel] [PATCH] avformat/oggparseogm: sync avctx w/ codecpar

I'm fine to do either. James, do you still prefer to skip the later headers if this breaks some old ogm files? On Sat, Feb 16, 2019 at 4:52 PM Michael Niedermayer wrote: > On Fri, Feb 15, 2019 at 02:56:18PM -0800, Chris Cunningham wrote: > > On Mon, Feb 11, 2019 at 1:55 PM Ch

Re: [FFmpeg-devel] [PATCH] avformat/oggparseogm: sync avctx w/ codecpar

On Mon, Feb 11, 2019 at 1:55 PM Chris Cunningham wrote: > On Fri, Feb 8, 2019 at 2:37 PM Michael Niedermayer > wrote: > >> ogg allows chaining streams when they have differing serial numbers >> https://xiph.org/ogg/doc/oggstream.html >> >> i think

Re: [FFmpeg-devel] [PATCH] avformat/oggparseogm: sync avctx w/ codecpar

On Fri, Feb 8, 2019 at 2:37 PM Michael Niedermayer wrote: > ogg allows chaining streams when they have differing serial numbers > https://xiph.org/ogg/doc/oggstream.html > > i think ive seen actual files doing this > > ogg_replace_stream() might assign these into existing avstreams i think > If

[FFmpeg-devel] [PATCH] avformat/oggparseogm: sync avctx w/ codecpar

Codec information may change while reading ogg packets. Update the stream's internal avctx to match. --- libavformat/oggparseogm.c | 3 +++ 1 file changed, 3 insertions(+) diff --git a/libavformat/oggparseogm.c b/libavformat/oggparseogm.c index a07453760b..b07a5d55ba 100644 --- a/libavformat/oggp

Re: [FFmpeg-devel] [PATCH] avformat/oggparseogm: sync avctx w/ codecpar

> developers dislike this: > (and i cannot edit other authors names of course, that would be not right) > > (from IRC) > Could you stop committing things like this? > his name is "Chris Cunningham", not "chcunningham" > > [...] > -- > Michael G

Re: [FFmpeg-devel] [PATCH] avformat/oggparseogm: sync avctx w/ codecpar

On Wed, Feb 6, 2019 at 6:03 PM James Almer wrote: > Can a valid ogm stream contain more than one header packet? Looking at ogg_packet oggdec.c, we read headers until hitting the first non-header (counted in ogg stream field nb_headers), so multiple headers per stream is probably ok. But multipl

Re: [FFmpeg-devel] [PATCH] avformat/mov: validate chunk_count vs stsc_data

Thanks! Looking at that file, I notice the stsc refers to some unknown chunks (stsco has no chunk offsets), but this is a non-issue since stts has no samples. Next patch will detect this condition and simply clear out stsc structures since they're not needed and contradict stco. On Wed, Feb 6, 201

Re: [FFmpeg-devel] [PATCH] avformat/mov.c: require tfhd to begin parsing trun

This will reject the file entirely. The testcase I have (can share once chromium bug is fixed) was previously hitting an av_assert0 in mov_read_trun, arguably also a total rejection ;). Recovery for this could be pretty tricky since the dropped trun will break decode dependencies. I would be surpri

Re: [FFmpeg-devel] [PATCH] avcodec/gsm_parser: return -1 on parse error

Thanks, this works great. Stand by for patch. On Wed, Feb 6, 2019 at 8:38 AM Hendrik Leppkes wrote: > On Thu, Jan 31, 2019 at 11:29 PM Chris Cunningham > wrote: > > > > On Wed, Jan 30, 2019 at 5:43 PM James Almer wrote: > > > > > On 1/30/2019

Re: [FFmpeg-devel] [PATCH] avformat/oggparseogm: sync avctx w/ codecpar

This is a follow up to feedback in http://ffmpeg.org/pipermail/ffmpeg-devel/2019-February/239751.html On Wed, Feb 6, 2019 at 5:13 PM chcunningham wrote: > Codec information may change while reading ogg packets. Update the > stream's internal avctx to match. > --- > libavformat/oggparseogm.c | 3

Re: [FFmpeg-devel] [PATCH] avcodec/gsm_parser: return -1 on parse error

> > The underlined bit above seems like the root cause. Where should we be > updating st->internal->avctx->codec_id? > Friendly ping for review ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

Re: [FFmpeg-devel] [PATCH] avformat/mov: validate chunk_count vs stsc_data

On Sat, Feb 2, 2019 at 3:55 AM Michael Niedermayer wrote: > > static inline int64_t mov_get_stsc_samples(MOVStreamContext *sc, > unsigned int index) > > { > > -int chunk_count; > > +unsigned int chunk_count = 0; > > > > if (mov_stsc_index_valid(index, sc->stsc_count)) > >

Re: [FFmpeg-devel] [PATCH] avformat/isom.h: use usnigned types in MOVStsc

On Sat, Feb 2, 2019 at 3:37 AM Michael Niedermayer wrote: > Is this change needed by some valid file ? > No, just a drive by fix since I happened to be looking at these types and the spec. > The change taken on its own is probably correct but its increasing the > range > of values without actu

Re: [FFmpeg-devel] [PATCH] avformat/mov: validate chunk_count vs stsc_data

Some extra context: this remedies some bad math that otherwise triggers an abort near the bottom of mov_seek_stream(). My first thought on seeing this was we should probably be returning AVERROR_INVALIDDATA somewhere. The stsc and stco boxes aren't agreeing (stco says no chunks, stsc.first points

Re: [FFmpeg-devel] [PATCH] avcodec/gsm_parser: return -1 on parse error

On Wed, Jan 30, 2019 at 5:43 PM James Almer wrote: > On 1/30/2019 10:20 PM, Chris Cunningham wrote: > >> > >>>> And this definitely means there's a bug elsewhere. This parser should > >>>> have not been used for codecs ids other than GSM and GSM_MS

Re: [FFmpeg-devel] [PATCH] avcodec/gsm_parser: return -1 on parse error

> > >> And this definitely means there's a bug elsewhere. This parser should > >> have not been used for codecs ids other than GSM and GSM_MS. That's > >> precisely what this assert() is making sure of. > >> > > > > I'll take a closer look at how this parser was selected. > Ok, here are full detai

Re: [FFmpeg-devel] [PATCH] avcodec/gsm_parser: return -1 on parse error

On Wed, Jan 30, 2019 at 1:40 PM James Almer wrote: > Parsers can't return negative values, only the output packet size. For > the purpose of errors, they usually return the entire untouched packet > size. > Understood. Is this documented somewhere? I noted the comment in av_paser_parse2(), "/* W

Re: [FFmpeg-devel] [PATCH] lavf/mov: ensure only one tkhd per trak

> > from a quick look, i did not find a file this breaks > Woot. Baptiste, I'm happy with this last patch if you are. ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

Re: [FFmpeg-devel] [PATCH] lavf/id3v2: fail read_apic on EOF reading mimetype

> > -char mimetype[64]; > +char mimetype[64] = {0}; > > would be enough > Agree! Next patch. I'd love to do similar for the isv34 branch, but I'm not sure how to detect > the condition given the EOF behavior for avio_get_str: > * @return number of bytes read (is always <= maxlen). > * If readi

Re: [FFmpeg-devel] [PATCH] lavf/mov: ensure only one tkhd per trak

s reasonably high. Still, I'll take a stab at the approach, if just for discussion/testing. On Thu, Dec 13, 2018 at 12:39 PM Chris Cunningham wrote: > "st->id" is not necessary for demuxing AFAIK, please correct me if Im >> wrong. >> Would an init value to

Re: [FFmpeg-devel] [PATCH] lavf/mov: ensure only one tkhd per trak

> > "st->id" is not necessary for demuxing AFAIK, please correct me if Im > wrong. > Would an init value to -1 for st->id work ? > st->id does get used here and there. For ex, mov_read_trun reads the id to decide which stream corresponds to the current fragment. But if the ID isn't coming from a t

Re: [FFmpeg-devel] [PATCH] lavf/mov: ensure only one tkhd per trak

> I have at least 2 files which have a id of 0 > Iam not sure where they are from so iam not sure i can share them This was my fear as well. Also, we currently default the ID for a new stream to be the number of streams now in the list. I worried that some files may lack a tkhd or could be structu

Re: [FFmpeg-devel] [PATCH] lavf/id3v2: fail read_apic on EOF reading mimetype

> > Yet another problem that could have been caught by static analysis.. > Wouldn't it be better to always leave the array in a valid state? > Will add that in the next patch. It has the extra benefit of protecting the isv34 branch. Goto fail; skips a lot of lines that aren't needed if mimetype i

Re: [FFmpeg-devel] [PATCH] avcodec/vp9_parser: export profile and pixel format

On Fri, Nov 2, 2018 at 4:57 PM Chris Cunningham wrote: > > Also, when dealing with a super frame, to get other values like pixel > > format and frame dimensions we'd have to make sure to parse the visible > > frame, and if it's an inter frame also keep the referen

Re: [FFmpeg-devel] [PATCH] avcodec/vp9_parser: export profile and pixel format

> Also, when dealing with a super frame, to get other values like pixel > format and frame dimensions we'd have to make sure to parse the visible > frame, and if it's an inter frame also keep the reference frames around > to take said values from them. Could we avoid the challenges around referenc

Re: [FFmpeg-devel] [PATCH] avcodec/vp9_parser: export profile and pixel format

facing codec private via extradata (reverting this change <http://ffmpeg.org/pipermail/ffmpeg-devel/2018-February/225548.html>)? On Thu, Oct 25, 2018 at 4:50 PM James Almer wrote: > On 10/24/2018 3:12 PM, Chris Cunningham wrote: > > On Wed, Oct 3, 2018 at 10:49 AM James Almer w

Re: [FFmpeg-devel] [PATCH] avcodec/vp9_parser: export profile and pixel format

On Wed, Oct 3, 2018 at 10:49 AM James Almer wrote: > Signed-off-by: James Almer > --- > libavcodec/vp9_parser.c | 82 - > 1 file changed, 80 insertions(+), 2 deletions(-) > > diff --git a/libavcodec/vp9_parser.c b/libavcodec/vp9_parser.c > index 9531f34a3

Re: [FFmpeg-devel] [PATCH] vp9_parser: set profile in AVCodecContext

On Tue, Oct 23, 2018 at 6:28 PM James Almer wrote: > On 10/23/2018 10:16 PM, chcunningham wrote: > > --- > > libavcodec/vp9_parser.c | 2 ++ > > 1 file changed, 2 insertions(+) > > > > diff --git a/libavcodec/vp9_parser.c b/libavcodec/vp9_parser.c > > index 9531f34a32..b6b621198b 100644 > > ---

[FFmpeg-devel] [PATCH] lavf/matroskadec: fix is_keyframe for early Blocks

Blocks are marked as key frames whenever the "reference" field is zero. This breaks for non-keyframe Blocks with a reference timestamp of zero. The likelihood of reference timestamp being zero is increased by a longstanding bug in muxing that encodes reference timestamp as the absolute time of the

Re: [FFmpeg-devel] [PATCH] lavf/matroskadec: fix is_keyframe for early Blocks

On Tue, Jan 31, 2017 at 10:18 PM, wm4 wrote: > On Tue, 31 Jan 2017 12:02:01 -0800 > Chris Cunningham wrote: > > > Thanks for taking a look. > > > > Definitely missing a "break;" - will fix in subsequent patch. > > > > Agree timestamps should

Re: [FFmpeg-devel] [PATCH] lavf/matroskadec: fix is_keyframe for early Blocks

On Tue, Jan 31, 2017 at 1:07 AM, wm4 wrote: > On Tue, 31 Jan 2017 09:57:24 +0100 > wm4 wrote: > > > On Mon, 30 Jan 2017 17:05:49 -0800 > > Chris Cunningham wrote: > > > > > Blocks are marked as key frames whenever the "reference" field is > &g

[FFmpeg-devel] [PATCH] lavf/matroskadec: fix is_keyframe for early Blocks

Blocks are marked as key frames whenever the "reference" field is zero. This is incorrect for non-keyframe Blocks that take a refernce on a keyframe at time zero. Now using -1 to denote "no reference". Reported to chromium at http://crbug.com/497889 (contains sample) --- libavformat/matroskadec.

Re: [FFmpeg-devel] [PATCH] libavformat/matroskadec: Add test for seeking with codec delay.

Thanks Michael. Do you mean to also apply the patch to add the test? Maybe you're waiting for further review. On Thu, Jul 28, 2016 at 9:48 AM, Michael Niedermayer wrote: > On Wed, Jul 27, 2016 at 06:35:50PM -0700, Chris Cunningham wrote: > > The file to upload to fate-suite can

Re: [FFmpeg-devel] [PATCH] libavformat/matroskadec: Add test for seeking with codec delay.

Thanks Michael. Do you mean to also apply the patch to add the test? Maybe you're waiting for further review. On Thu, Jul 28, 2016 at 9:48 AM, Michael Niedermayer wrote: > On Wed, Jul 27, 2016 at 06:35:50PM -0700, Chris Cunningham wrote: > > The file to upload to fate-suite can

Re: [FFmpeg-devel] [PATCH] libavformat/matroskadec: Add test for seeking with codec delay.

: > From: Chris Cunningham > > Also cleanup parens for the skip_to_timecode check. > --- > libavformat/matroskadec.c | 2 +- > tests/fate/seek.mak| 3 +++ > tests/ref/seek/mkv-codec-delay | 48 > ++ > 3 files c

Re: [FFmpeg-devel] [PATCH] libavformat/matroskadec: fix unsigned overflow to improve seeking

Happy to make a test. Stand by for patch. On Sat, Jul 23, 2016 at 2:26 AM, Michael Niedermayer wrote: > On Thu, Jul 21, 2016 at 12:01:45PM -0700, Chris Cunningham wrote: > > When seeking a file where codec delay is greater than 0, the timecode > > can become negative after of

Re: [FFmpeg-devel] [PATCH] libavformat/matroskadec: fix unsigned overflow to improve seeking

find that the playback begins from the 2 second time. On Thu, Jul 21, 2016 at 12:01 PM, Chris Cunningham < chcunning...@chromium.org> wrote: > When seeking a file where codec delay is greater than 0, the timecode > can become negative after offsetting by the codec delay. Failing to

[FFmpeg-devel] [PATCH] libavformat/matroskadec: fix unsigned overflow to improve seeking

When seeking a file where codec delay is greater than 0, the timecode can become negative after offsetting by the codec delay. Failing to cast to a signed int64 will cause the check against skip_to_timecode to evaluate true for these negative values. This breaks the "skip_to" seek mechanism. --- l

Re: [FFmpeg-devel] [PATCH] avformat/utils: Check negative bps before shifting in ff_get_pcm_codec_id()

New patch drops the U. Hendrik, you'r right about the negative, but you're also right about the 0. 0 is a case of undefined shift behavior (shift becomes -1). On Tue, May 17, 2016 at 11:28 AM, wrote: > From: Chris Cunningham > > Fixes: undefined shift. > --- > libavf

[FFmpeg-devel] [PATCH] libavformat/oggdec: Free stream private when header parsing fails.

Leaking this private structure opens up the possibility that it may be re-used when parsing later packets in the stream. This is problematic if the later packets are not the same codec type (e.g. private allocated during Vorbis parsing, but later packets are Opus and the private is assumed to be th

Re: [FFmpeg-devel] size=0, but av_malloc(1)

ed, Mar 23, 2016 at 1:56 PM, Reimar Döffinger wrote: > On Wed, Mar 23, 2016 at 03:31:38PM +0100, Michael Niedermayer wrote: > > On Tue, Mar 22, 2016 at 11:43:50PM -0700, Chris Cunningham wrote: > > > Hey Group, > > > > > > I'm seeing an interesting pattern [0][

[FFmpeg-devel] size=0, but av_malloc(1)

Hey Group, I'm seeing an interesting pattern [0][1] where we allocate 1 byte in places where I would expect no allocation to be necessary. Why is this being done? [0] https://github.com/FFmpeg/FFmpeg/blob/master/libavutil/mem.c#L136 [1] https://github.com/FFmpeg/FFmpeg/blob/master/libavformat/ogg

Re: [FFmpeg-devel] [PATCH] avformat/mp3dec: prefer "fast_seek" to TOC seek for CBR files.

Thanks wm4, next patch will fix that declaration. Michael, any concerns? On Mon, Nov 30, 2015 at 2:51 PM, wm4 wrote: > On Mon, 30 Nov 2015 14:29:50 -0800 > chcunning...@chromium.org wrote: > > > From: Chris Cunningham > > > > "Fast seek" uses linear inte

Re: [FFmpeg-devel] [PATCH] avformat/mp3dec: prefer "fast_seek" to TOC seek for CBR files.

Looks like I messed up the tab for "return best_pos;"... fix incoming. On Mon, Nov 30, 2015 at 2:10 PM, wrote: > From: Chris Cunningham > > "Fast seek" uses linear interpolation to find the position of the > requested seek time. For CBR this is more dir

Re: [FFmpeg-devel] [PATCH] avformat/mp3dec: prefer "fast_seek" to TOC seek for CBR files.

(Fixed CCs) Thanks for review. Patch coming shortly. On Thu, Nov 26, 2015 at 10:13 AM, wm4 wrote: > On Tue, 24 Nov 2015 16:55:03 -0800 > chcunning...@chromium.org wrote: > > > From: Chris Cunningham > > > > "Fast seek" uses linear interpolation to find t

Re: [FFmpeg-devel] [PATCH] avformat/mp3dec: prefer "fast_seek" to TOC seek for CBR files.

29). I think I can restore the behavior of this test to use the av_rescale path if I replace -usetoc 0 with -fflags fastseek (and update args parsing)... is that what we want? Thanks, Chris On Tue, Nov 24, 2015 at 4:55 PM, wrote: > From: Chris Cunningham > > "Fast seek" uses

Re: [FFmpeg-devel] [PATCH] avformat/mp3dec: prefer "fast_seek" to TOC seek for CBR files.

large files. I break down what I know about the design in this bug: https://code.google.com/p/chromium/issues/detail?id=545914#c13 - LMK if I'm missing something. On Wed, Nov 18, 2015 at 3:08 AM, wm4 wrote: > On Tue, 17 Nov 2015 14:21:08 -0800 > Chris Cunningham wrote: > > > On

Re: [FFmpeg-devel] [PATCH] avformat/mp3dec: prefer "fast_seek" to TOC seek for CBR files.

r. ./ffplay testcount.mp3 -ss 00:33:20 -fflags fastseek *is changed: *plays out "1395", which is correct. fast seek no longer uses the TOC for CBR files. What do you think? I personally prefer this way so that usetoc is still meaningful for CBR files. Thanks, Chris On Mon, Nov 16,

Re: [FFmpeg-devel] [PATCH] avformat/mp3dec: prefer "fast_seek" to TOC seek for CBR files.

for a second patch that will only change TOC behavior when the AVFMT_FLAG_FAST_SEEK is being used. Chris On Mon, Nov 9, 2015 at 3:34 PM, Michael Niedermayer wrote: > On Wed, Nov 04, 2015 at 05:21:57PM -0800, Chris Cunningham wrote: > > "Fast seek" uses linear interpolation

[FFmpeg-devel] [PATCH] avformat/mp3dec: prefer "fast_seek" to TOC seek for CBR files.

"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 when the TOC is corrupted (e.g. https://crbug.com/545914). For VBR, fast seek is not precise, so continue to prefer the TO

[FFmpeg-devel] chromium-review spam - my mistake

Hey Group, Please ignore a wave of emails that were just sent out asking for code review at https://chromium-review.googlesource.com/ . Sorry for the spam! I'm rolling upstream ffmpeg into chromium and our review tools have changed recently. Our d