After an evaluation, GNOME has moved from Bugzilla to GitLab. Learn more about GitLab.
No new issues can be reported in GNOME Bugzilla anymore.
To report an issue in a GNOME project, go to GNOME GitLab.
Do not go to GNOME Gitlab for: Bluefish, Doxygen, GnuCash, GStreamer, java-gnome, LDTP, NetworkManager, Tomboy.
Bug 787795 - flvdemux: unable to handle file that works fine in ffmpeg
flvdemux: unable to handle file that works fine in ffmpeg
Status: RESOLVED FIXED
Product: GStreamer
Classification: Platform
Component: gst-plugins-good
1.x
Other Linux
: Normal normal
: 1.12.4
Assigned To: GStreamer Maintainers
GStreamer Maintainers
Depends on:
Blocks:
 
 
Reported: 2017-09-17 17:41 UTC by Nicola
Modified: 2017-10-19 14:00 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
test file (1.22 MB, video/x-flv)
2017-09-17 17:41 UTC, Nicola
  Details
avdemux: enable flv demuxer (880 bytes, patch)
2017-09-23 15:50 UTC, Nicola
rejected Details | Review
avdemux: reset to 0 negative pts (1.54 KB, patch)
2017-09-23 15:52 UTC, Nicola
committed Details | Review
flvdemux: Avoid integer overflow on invalid CTS (1.04 KB, patch)
2017-09-23 19:47 UTC, Nicolas Dufresne (ndufresne)
committed Details | Review
flvdemux: Ignore invalid H.264 codec data (1.18 KB, patch)
2017-09-23 19:48 UTC, Nicolas Dufresne (ndufresne)
committed Details | Review
flvdemux: Don't pull passed the EOS (982 bytes, patch)
2017-09-23 19:48 UTC, Nicolas Dufresne (ndufresne)
committed Details | Review
better test file (1.97 MB, video/x-flv)
2017-09-24 12:57 UTC, Nicola
  Details
flvdemux: Only set pixel-aspect-ratio if specified (1.13 KB, patch)
2017-09-24 18:36 UTC, Nicolas Dufresne (ndufresne)
committed Details | Review

Description Nicola 2017-09-17 17:41:28 UTC
Created attachment 359936 [details]
test file

flvdemux seems unable to handle this file that ffmpeg/vlc can handle just fine,

probably the demuxer is confused since there are some negative cts at the beginning that generate really wrong timestamps
Comment 1 Nicola 2017-09-17 17:59:44 UTC
ffplay shows warning such as this

[flv @ 0x7f59b4000b40] Negative cts, previous timestamps might be wrong.
[flv @ 0x7f59b4000b40] Invalid timestamps stream=0, pts=-4719, dts=0, size=5006
[flv @ 0x7f59b4000b40] Invalid timestamps stream=0, pts=-4799, dts=0, size=897
[flv @ 0x7f59b4000b40] Invalid timestamps stream=0, pts=-4759, dts=0, size=779
[flv @ 0x7f59b4000b40] Invalid timestamps stream=0, pts=-4599, dts=0, size=3594

but it plays the file just fine, flvdemux generate timestamps such this one:

gstflvdemux.c:1638:gst_flv_demux_parse_tag_video:<flvdemux0> dts 0 pts 4294962577 cts -4719

and then skips audio:

Signalling no-more-pads because no audio stream was found after 6 seconds of video

but video cannot the decoded as well:

gstpad.c:5478:pre_eventfunc_check:<avdec_h264-0:sink> caps video/x-h264, stream-format=(string)avc, pixel-aspect-ratio=(fraction)1/1, width=(int)540, height=(int)304, framerate=(fraction)25/1, codec_data=(buffer)0d not accepted

probably because alignment is missing,

please note that pixel aspect ratio is wrong too, it should be 1216/1215 as reported by ffmpeg and not 1/1
Comment 2 Nicola 2017-09-18 08:31:18 UTC
enabling avdemux_flv in gst-libav and applying these small modifications/hacks:

1) here: https://cgit.freedesktop.org/gstreamer/gst-libav/tree/ext/libav/gstavdemux.c#n1424 use pkt.dts if pkt.pts < 0
2) here: https://cgit.freedesktop.org/gstreamer/gst-libav/tree/ext/libav/gstavcodecmap.c#n1244 adding stream-format avc to the caps

make this pipeline works as expected:

gst-launch-1.0 -v filesrc location=/tmp/test.flv ! avdemux_flv name=demux demux.video_0 ! queue ! avdec_h264 ! xvimagesink demux.audio_0 ! queue ! aacparse ! avdec_aac ! pulsesink

here is the verbose gst-launch output with useful info about the right codec data ecc..

/GstPipeline:pipeline0/GstQueue:queue0.GstPad:sink: caps = video/x-h264, width=(int)540, height=(int)304, framerate=(fraction)25/1, alignment=(string)au, stream-format=(string)avc, codec_data=(buffer)014d001effe10024674d401e965604413f7ffe0980097e9100000300010000030032e8c00ac00158af7be0a001000568ef060cc8
/GstPipeline:pipeline0/GstQueue:queue0.GstPad:src: caps = video/x-h264, width=(int)540, height=(int)304, framerate=(fraction)25/1, alignment=(string)au, stream-format=(string)avc, codec_data=(buffer)014d001effe10024674d401e965604413f7ffe0980097e9100000300010000030032e8c00ac00158af7be0a001000568ef060cc8
/GstPipeline:pipeline0/GstQueue:queue1.GstPad:sink: caps = audio/mpeg, rate=(int)44100, channels=(int)2, channel-mask=(bitmask)0x0000000000000003, mpegversion=(int)4, stream-format=(string)raw, base-profile=(string)lc, level=(string)1, profile=(string)lc, codec_data=(buffer)1390
/GstPipeline:pipeline0/GstQueue:queue1.GstPad:src: caps = audio/mpeg, rate=(int)44100, channels=(int)2, channel-mask=(bitmask)0x0000000000000003, mpegversion=(int)4, stream-format=(string)raw, base-profile=(string)lc, level=(string)1, profile=(string)lc, codec_data=(buffer)1390
/GstPipeline:pipeline0/GstAacParse:aacparse0.GstPad:src: caps = audio/mpeg, rate=(int)22050, channels=(int)2, channel-mask=(bitmask)0x0000000000000003, mpegversion=(int)4, stream-format=(string)raw, base-profile=(string)lc, level=(string)1, profile=(string)lc, codec_data=(buffer)1390, framed=(boolean)true
Redistribute latency...
/GstPipeline:pipeline0/avdec_h264:avdec_h264-0.GstPad:sink: caps = video/x-h264, width=(int)540, height=(int)304, framerate=(fraction)25/1, alignment=(string)au, stream-format=(string)avc, codec_data=(buffer)014d001effe10024674d401e965604413f7ffe0980097e9100000300010000030032e8c00ac00158af7be0a001000568ef060cc8
/GstPipeline:pipeline0/avdec_aac:avdec_aac0.GstPad:sink: caps = audio/mpeg, rate=(int)22050, channels=(int)2, channel-mask=(bitmask)0x0000000000000003, mpegversion=(int)4, stream-format=(string)raw, base-profile=(string)lc, level=(string)1, profile=(string)lc, codec_data=(buffer)1390, framed=(boolean)true
/GstPipeline:pipeline0/GstAacParse:aacparse0.GstPad:sink: caps = audio/mpeg, rate=(int)44100, channels=(int)2, channel-mask=(bitmask)0x0000000000000003, mpegversion=(int)4, stream-format=(string)raw, base-profile=(string)lc, level=(string)1, profile=(string)lc, codec_data=(buffer)1390
/GstPipeline:pipeline0/avdec_aac:avdec_aac0.GstPad:src: caps = audio/x-raw, format=(string)F32LE, layout=(string)interleaved, rate=(int)44100, channels=(int)2, channel-mask=(bitmask)0x0000000000000003
Redistribute latency...
/GstPipeline:pipeline0/GstPulseSink:pulsesink0.GstPad:sink: caps = audio/x-raw, format=(string)F32LE, layout=(string)interleaved, rate=(int)44100, channels=(int)2, channel-mask=(bitmask)0x0000000000000003
/GstPipeline:pipeline0/avdec_h264:avdec_h264-0.GstPad:src: caps = video/x-raw, format=(string)I420, width=(int)540, height=(int)304, interlace-mode=(string)progressive, multiview-mode=(string)mono, multiview-flags=(GstVideoMultiviewFlagsSet)0:ffffffff:/right-view-first/left-flipped/left-flopped/right-flipped/right-flopped/half-aspect/mixed-mono, pixel-aspect-ratio=(fraction)1216/1215, chroma-site=(string)mpeg2, colorimetry=(string)bt601, framerate=(fraction)25/1
/GstPipeline:pipeline0/GstXvImageSink:xvimagesink0.GstPad:sink: caps = video/x-raw, format=(string)I420, width=(int)540, height=(int)304, interlace-mode=(string)progressive, multiview-mode=(string)mono, multiview-flags=(GstVideoMultiviewFlagsSet)0:ffffffff:/right-view-first/left-flipped/left-flopped/right-flipped/right-flopped/half-aspect/mixed-mono, pixel-aspect-ratio=(fraction)1216/1215, chroma-site=(string)mpeg2, colorimetry=(string)bt601, framerate=(fraction)25/1


fixing flvdemux would be better but it seems not easy as enabling/fixing, using avdemux_flv
Comment 3 Nicola 2017-09-23 15:50:15 UTC
Created attachment 360304 [details] [review]
avdemux: enable flv demuxer
Comment 4 Nicola 2017-09-23 15:52:14 UTC
Created attachment 360305 [details] [review]
avdemux: reset to 0 negative pts

this can happen for some rtmp streams and gstreamer timestamp must be positive, another option could be to add a fixed offset and so make the timestamp don't start from 0
Comment 5 Nicolas Dufresne (ndufresne) 2017-09-23 17:08:56 UTC
Have you run gst-validate, usually the ffmpeg demuxer don't pass all the seeking cases.

I still think it would be better to fix the flvdemux element. According to the trace you provided you do have negative value it's just that you get a big number (4294962577) due to the usage of unsigned integer. Fixing such overflow seems like a good idea, and shouldn't be very hard.
Comment 6 Tim-Philipp Müller 2017-09-23 17:19:03 UTC
Yes, we should fix flvdemux. I don't think we should re-enable avdemux_flv.
Comment 7 Nicolas Dufresne (ndufresne) 2017-09-23 19:47:59 UTC
Created attachment 360308 [details] [review]
flvdemux: Avoid integer overflow on invalid CTS

If the CTS is negative an would lead to a negtive PTS, clip
the CTS so the PTS will be 0.
Comment 8 Nicolas Dufresne (ndufresne) 2017-09-23 19:48:03 UTC
Created attachment 360309 [details] [review]
flvdemux: Ignore invalid H.264 codec data

This code basically skip over codec_data with empty payload. In
this case, the codec_data variable is the size of the header for
the CODEC part of Video Tag. The remaining is supposed to be the
H.264 codec data, hence should not be empty.
Comment 9 Nicolas Dufresne (ndufresne) 2017-09-23 19:48:07 UTC
Created attachment 360310 [details] [review]
flvdemux: Don't pull passed the EOS

When a truncated FLV is provided and processed in pull mode, we
may endup trying to pull passed EOS, causing a rather confusing
warning as the pull offset is an integer overflow.
Comment 10 Nicolas Dufresne (ndufresne) 2017-09-23 19:54:08 UTC
These 3 patches make it go further, but the video is gray. These are massive errors in the FLV container, so I'm a bit suspicious that we may endup off by one from time to time (on some tags). We need to notice that h264parse is pretty unhappy with the codec_data and I wonder if it's not the reason the decoding is gray (e.g. ffmpeg doing better guesses on how to fix it).

Unlike the initial report, ffplay does not seem to really play correctly, unless this video is really just 1 still frame repeated ? Would be nice to set the expectation by the way.
Comment 11 Nicolas Dufresne (ndufresne) 2017-09-23 19:55:20 UTC
Review of attachment 360305 [details] [review]:

I think it's fine to merge this one.
Comment 12 Nicolas Dufresne (ndufresne) 2017-09-23 19:56:12 UTC
Review of attachment 360304 [details] [review]:

We'd need to prove that it works well enough first, not sure we really want this though, rejecting, but it's trivial patch anyway.
Comment 13 Nicola 2017-09-23 22:33:46 UTC
thanks Nicolas,

after applying your patchest we now have the same behaviour as with avdemux_flv, a pipeline like this one:

filesrc location=/tmp/test.flv ! flvdemux ! video/x-h264,alignment=au ! avdec_h264 ! fakesink

works,

the capsfilter is needed since flvdemux does not add alignment=au to the caps and h264parse will broke the stream (as noted by you too)

regarding ffplay I can confirm that it plays the rtmp streams correctly, I tested with not repeating video too and it has no issues.

anyway flvdemux seems unable to detect audio, please try this:

filesrc ! avdemux_flv name=d d.audio_0 ! fakesink

it detects

caps = audio/mpeg, rate=(int)44100, channels=(int)2, channel-mask=(bitmask)0x0000000000000003, mpegversion=(int)4, stream-format=(string)raw, base-profile=(string)lc, level=(string)1, profile=(string)lc, codec_data=(buffer)1390

while this pipeline:

filesrc ! avdemux_flv name=d d.audio ! fakesink

will not detects audio
Comment 14 Nicola 2017-09-23 22:43:51 UTC
the pipeline that does not works with audio is

filesrc ! flvdemux name=d d.audio ! fakesink

anyway if I remove d.audio the logs shows that an audio pad is detected

gstflvdemux.c:906:gst_flv_demux_audio_negotiate:<'':audio> successfully negotiated caps audio/mpeg, mpegversion=(int)4, framed=(boolean)true, stream-format=(string)raw, rate=(int)22050, channels=(int)2, codec_data=(buffer)1390

the rate seems wrong, the right one should be 44100 as detected by avdemux_flv and ffplay
Comment 15 Nicola 2017-09-23 22:55:49 UTC
this pipeline is fine

gst-launch-1.0 -v filesrc location=/tmp/test.flv ! flvdemux name=d d.video ! queue ! fakesink d.audio ! queue ! fakesink

but as noted in the previous comment the audio caps seems wrong
Comment 16 Nicola 2017-09-24 12:57:53 UTC
Created attachment 360317 [details]
better test file

with changing video and real audio
Comment 17 Nicola 2017-09-24 13:01:50 UTC
the latest test file works with this pipeline

gst-launch-1.0 -v filesrc location=/tmp/test1.flv ! flvdemux name=d d.video ! queue max-size-time=0 ! video/x-h264,alignment=au ! avdec_h264 ! xvimagesink d.audio ! queue max-size-time=0 ! aacparse ! avdec_aac ! pulsesink

after applying the patch here,

if you add h264parse before avdec_h264 you'll get grey video
Comment 18 Nicolas Dufresne (ndufresne) 2017-09-24 13:25:12 UTC
(In reply to Nicola from comment #13)
> thanks Nicolas,
> 
> after applying your patchest we now have the same behaviour as with
> avdemux_flv, a pipeline like this one:
> 
> filesrc location=/tmp/test.flv ! flvdemux ! video/x-h264,alignment=au !
> avdec_h264 ! fakesink
> 

Wow, ok, thanks for spotting, so h264parse needs help too :-(

> works,
> 
> the capsfilter is needed since flvdemux does not add alignment=au to the
> caps and h264parse will broke the stream (as noted by you too)
> 
> regarding ffplay I can confirm that it plays the rtmp streams correctly, I
> tested with not repeating video too and it has no issues.
> 
> anyway flvdemux seems unable to detect audio, please try this:
> 
> filesrc ! avdemux_flv name=d d.audio_0 ! fakesink
> 
> it detects
> 
> caps = audio/mpeg, rate=(int)44100, channels=(int)2,
> channel-mask=(bitmask)0x0000000000000003, mpegversion=(int)4,
> stream-format=(string)raw, base-profile=(string)lc, level=(string)1,
> profile=(string)lc, codec_data=(buffer)1390
> 
> while this pipeline:
> 
> filesrc ! avdemux_flv name=d d.audio ! fakesink

I guess you mean flvdemux here.

> 
> will not detects audio

Ok.
Comment 19 Nicola 2017-09-24 13:31:28 UTC
(In reply to Nicolas Dufresne (stormer) from comment #18)
> (In reply to Nicola from comment #13)
> > thanks Nicolas,
> > 
> > after applying your patchest we now have the same behaviour as with
> > avdemux_flv, a pipeline like this one:
> > 
> > filesrc location=/tmp/test.flv ! flvdemux ! video/x-h264,alignment=au !
> > avdec_h264 ! fakesink
> > 
> 
> Wow, ok, thanks for spotting, so h264parse needs help too :-(
> 
> > works,
> > 
> > the capsfilter is needed since flvdemux does not add alignment=au to the
> > caps and h264parse will broke the stream (as noted by you too)
> > 
> > regarding ffplay I can confirm that it plays the rtmp streams correctly, I
> > tested with not repeating video too and it has no issues.
> > 
> > anyway flvdemux seems unable to detect audio, please try this:
> > 
> > filesrc ! avdemux_flv name=d d.audio_0 ! fakesink
> > 
> > it detects
> > 
> > caps = audio/mpeg, rate=(int)44100, channels=(int)2,
> > channel-mask=(bitmask)0x0000000000000003, mpegversion=(int)4,
> > stream-format=(string)raw, base-profile=(string)lc, level=(string)1,
> > profile=(string)lc, codec_data=(buffer)1390
> > 
> > while this pipeline:
> > 
> > filesrc ! avdemux_flv name=d d.audio ! fakesink
> 
> I guess you mean flvdemux here.

yes, but this is not a big problem, the pipeline in comment 17 works

> 
> > 
> > will not detects audio
> 
> Ok.
Comment 20 Nicolas Dufresne (ndufresne) 2017-09-24 13:38:34 UTC
A right, well, it's a miss-behaviour of the demuxer, but the parsing looks fine.
Comment 21 Nicolas Dufresne (ndufresne) 2017-09-24 13:41:49 UTC
And do we still have a bug with detected PAR ? I have not looked at the yet but you said it should be "it should be 1216/1215"
Comment 22 Nicola 2017-09-24 13:58:25 UTC
(In reply to Nicolas Dufresne (stormer) from comment #21)
> And do we still have a bug with detected PAR ? I have not looked at the yet
> but you said it should be "it should be 1216/1215"

flvdemux outputs these video caps:

video/x-h264, alignment=(string)au, stream-format=(string)avc, pixel-aspect-ratio=(fraction)1/1, width=(int)540, height=(int)304, framerate=(fraction)25/1, codec_data=(buffer)014d001effe10024674d401e965604413f7ffe0980097e9100000300010000030032e8c00ac00158af7be0a001000568ef060cc8

using avdec_h264 after flvdemux we have:

caps = video/x-raw, format=(string)I420, width=(int)540, height=(int)304, interlace-mode=(string)progressive, multiview-mode=(string)mono, multiview-flags=(GstVideoMultiviewFlagsSet)0:ffffffff:/right-view-first/left-flipped/left-flopped/right-flipped/right-flopped/half-aspect/mixed-mono, pixel-aspect-ratio=(fraction)1216/1215, chroma-site=(string)mpeg2, colorimetry=(string)bt601, framerate=(fraction)25/1

and the PAR is now ok, 

something similar happens with the audio caps, flvdemux detect them as


caps = audio/mpeg, mpegversion=(int)4, framed=(boolean)true, stream-format=(string)raw, rate=(int)22050, channels=(int)2, codec_data=(buffer)1390

while avdemux_flv detects

caps = audio/mpeg, rate=(int)44100, channels=(int)2, channel-mask=(bitmask)0x0000000000000003, mpegversion=(int)4, stream-format=(string)raw, base-profile=(string)lc, level=(string)1, profile=(string)lc, codec_data=(buffer)1390

but using something like this:

flvdemux name=d d.audio ! queue ! aacparse ! avdec_aac ! ...

we these caps:

avdec_aac0.GstPad:src: caps = audio/x-raw, format=(string)F32LE, layout=(string)interleaved, rate=(int)44100, channels=(int)2, channel-mask=(bitmask)0x0000000000000003

and the audio seems fine,

you can test both video and audio with the latest file I uploaded using the pipeline in comment 17
Comment 23 Nicolas Dufresne (ndufresne) 2017-09-24 18:36:28 UTC
Created attachment 360320 [details] [review]
flvdemux: Only set pixel-aspect-ratio if specified

If it's not specified, we should let the decoder figure it out.
Apparently the code was already in place, all was to make the code
conditional.
Comment 24 Nicolas Dufresne (ndufresne) 2017-09-24 18:51:44 UTC
That fixes the PAR issue, it was forcing a PAR even if there was no associated tag. It's better to delegate to the decoder in this case.

For the case where we only have an audio pad pending, I found that the flow aggregation is broken in the demuxer. Basically, it returns not-linked before the audio pad has been made available. I'll see if I can fix this later. It's relatively minor, but still, pretty annoying behaviour.

Finally, for the audio rate, well, this is what you get when you try and workaround Youtube bugs. That happened in 2010, https://bugzilla.gnome.org/show_bug.cgi?id=636621 . A lot of youtube streams had samplerate tag in the container that was wrong. So what we have now, is code that parses the AAC header. While the code is correct, it does not detect that this is HE-AAC and that the declared rate need to be doubled. I don't know myself how to parse that, will need research. aacparse does not seem to fix it / detect it either.
Comment 25 Nicolas Dufresne (ndufresne) 2017-09-24 18:55:03 UTC
Maybe I should mention too, that according to the FLV specification, only stereo 44.1 kHz audio is tolerated. I'm not too fan of limiting that, since I'm sure many users are cheating.
Comment 26 Nicola 2017-09-24 19:06:50 UTC
Thanks Nicolas!

The only remaining issue here is the h264 caps without alignment=au and/or h264parse that is unable to parse them.

Meantime I developed an rtmp source element based on ffmpeg to be able to receive this stream (rtmpsrc segfaults on exit as you probably know), if this can be of general interest I can make it public (anyway for this element to work it needs libav in gst-libav recompiled with network and protocols enabled, they are currently explicitly disabled)
Comment 27 Nicolas Dufresne (ndufresne) 2017-09-24 19:31:30 UTC
Yep, agreed. The aac minor issue is well known:

https://bugzilla.gnome.org/show_bug.cgi?id=771994

For H264 parse failure, I'll soon file a seperate bug. For RTPM there is an alternative here:

https://bugzilla.gnome.org/show_bug.cgi?id=783354
Comment 28 Nicola 2017-09-24 20:38:03 UTC
(In reply to Nicolas Dufresne (stormer) from comment #27)
> Yep, agreed. The aac minor issue is well known:
> 
> https://bugzilla.gnome.org/show_bug.cgi?id=771994
> 
> For H264 parse failure, I'll soon file a seperate bug. For RTPM there is an
> alternative here:
> 
> https://bugzilla.gnome.org/show_bug.cgi?id=783354

I tested this alternative implementation the last weekend and it compliants about not supporting abobe authentication when I point rtmp2src to my stream, since ffmpeg has it own implementation too (and it works fine with my stream) maybe we can use that one instead of starting from scratch again ... this is the main reason I wrote this new element (that basically open the stream and call avio_read on src_create, very few line of code)
Comment 29 Nicolas Dufresne (ndufresne) 2017-10-06 01:22:43 UTC
So the remaining bug is in h264parse. Leaving open.

Attachment 360308 [details] pushed as 18dbd49 - flvdemux: Avoid integer overflow on invalid CTS
Attachment 360309 [details] pushed as cfc1638 - flvdemux: Ignore invalid H.264 codec data
Attachment 360310 [details] pushed as 307018d - flvdemux: Don't pull passed the EOS
Attachment 360320 [details] pushed as 986f3e1 - flvdemux: Only set pixel-aspect-ratio if specified
Comment 30 Nicolas Dufresne (ndufresne) 2017-10-06 01:42:03 UTC
Attachment 360305 [details] pushed as f86dfde - avdemux: reset to 0 negative pts
Comment 31 Nicola 2017-10-06 06:29:28 UTC
thanks Nicolas, the bug status is now "RESOLVED", do you want I open another one for h264parse issue?
Comment 32 Nicolas Dufresne (ndufresne) 2017-10-06 12:27:52 UTC
Oops, tat was accidental, but I'll open a different issue, it's a good idea.
Comment 33 Nicolas Dufresne (ndufresne) 2017-10-06 12:33:50 UTC
Created https://bugzilla.gnome.org/show_bug.cgi?id=788595