[Bug 753907] New: asfdemux: skip a frame with an abnormal time stamp.

["GStreamer" (GNOME Bugzilla)]

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

Bug ID: 753907
Summary: asfdemux: skip a frame with an abnormal time stamp.
Classification: Platform
Product: GStreamer
Version: git master
OS: Windows
Status: NEW
Severity: normal
Priority: Normal
Component: gst-plugins-ugly
Assignee: gstreamer-***@lists.freedesktop.org
Reporter: ***@gmail.com
QA Contact: gstreamer-***@lists.freedesktop.org
GNOME version: ---

Created attachment 309786
--> https://bugzilla.gnome.org/attachment.cgi?id=309786&action=edit
patch to resolve the issue

Although a demuxer succeeded to parse a frame, the frame can be defective.
So I added some codes for the asf demuxer to skip frames with an normal
timestamp.
I regarded the timestamp is abnormal in case the timestamp is out of the
segment duration.
Issue can be observed with the attached test stream and can be resolved with
the attached patch.
Kindly share your views/concerns on this.

["GStreamer" (GNOME Bugzilla)]

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

Sebastian Dröge (slomo) <***@coaxion.net> changed:

What |Removed |Added
----------------------------------------------------------------------------
Attachment #309786|none |needs-work
status| |

--- Comment #1 from Sebastian Dröge (slomo) <***@coaxion.net> ---
Review of attachment 309786:
--> (https://bugzilla.gnome.org/review?bug=753907&attachment=309786)

Please provide a "git format-patch" style patch :)

::: gst/asfdemux/gstasfdemux.c
@@ +1742,3 @@
+ GST_TIME_ARGS (GST_BUFFER_PTS (payload->buf)),
+ GST_TIME_ARGS (demux->segment.duration));
+ gst_buffer_unref (payload->buf);

It might be a good idea to explicitly set ret = GST_FLOW_OK here. Also there
might be cases where the duration is reported wrong, and there are actually
valid buffers after the reported duration. This might break such streams.

@@ +1744,3 @@
+ gst_buffer_unref (payload->buf);
+ } else
+ ret = gst_pad_push (stream->pad, payload->buf);

Add some {} around here

["GStreamer" (GNOME Bugzilla)]

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

--- Comment #2 from Rajesh Singh <***@gmail.com> ---
Dear Sebastian,

Thanks for your review comments.

I think under no scenario, we should push a payload having PTS more than the
segment duration.
For normal playback, segment duration is the play time of media file, which we
parse from the stream property object.

I have a test stream, to test this scenario, and i found that, video freezes,
if we are not skipping/dropping the abnormal payload.

Kindly share your views/concerns on this

["GStreamer" (GNOME Bugzilla)]

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

Rajesh Singh <***@gmail.com> changed:

What |Removed |Added
----------------------------------------------------------------------------
CC| |***@gmail.com

["GStreamer" (GNOME Bugzilla)]

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

Sebastian Dröge (slomo) <***@coaxion.net> changed:

What |Removed |Added
----------------------------------------------------------------------------
CC| |***@coaxion.net

--- Comment #3 from Sebastian Dröge (slomo) <***@coaxion.net> ---
The problem is that the duration information inside the stream might be wrong.
It might say 5s duration, while the stream actually has a few minutes duration.

["GStreamer" (GNOME Bugzilla)]

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

--- Comment #4 from Rajesh Singh <***@gmail.com> ---
Dear Sebastian,

In ASFdemux, we are sending a segment event, specifying the start, stop, rate,
duration and flags.

if we see the code in function, gst_asf_demux_process_file():

play_time = gst_asf_demux_get_uint64 (&data, &size);
and if we are getting a valid play_time and we are updating the duration.
(if we get 5sec, we treat it as valid duration)

if (demux->play_time > 0) {
demux->segment.duration = demux->play_time;
}

As per my understanding, gstreamer will render the payloads only within this
segment duration value.


Kindly share your views/concerns on this.

Regards:
Rajesh Singh

["GStreamer" (GNOME Bugzilla)]

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

Tim-Philipp Müller <***@zen.co.uk> changed:

What |Removed |Added
----------------------------------------------------------------------------
CC| |***@zen.co.uk

--- Comment #5 from Tim-Philipp Müller <***@zen.co.uk> ---
What is the reason for this defect?

["GStreamer" (GNOME Bugzilla)]

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

--- Comment #6 from Rajesh Singh <***@gmail.com> ---
video freezes, while audio continues to play.

["GStreamer" (GNOME Bugzilla)]

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

--- Comment #7 from Tim-Philipp Müller <***@zen.co.uk> ---
I meant, why is the timestamp not correct, is it because whatever generated the
file messed up, or because it got corrupted during transport (how/where/why?)?

["GStreamer" (GNOME Bugzilla)]

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

--- Comment #8 from Rajesh Singh <***@gmail.com> ---

The generated file is messed up, but skipping/dropping the payload with
abnormal TS, will resolve this video freezing issue.

["GStreamer" (GNOME Bugzilla)]

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

--- Comment #9 from Rajesh Singh <***@gmail.com> ---
Created attachment 310775
--> https://bugzilla.gnome.org/attachment.cgi?id=310775&action=edit
This is diff-patch after modification

Once we sent the segment event downstream, pushing any payload out of that
segment is not justified. If we send, video freeze is observed, although audio
is playing.

["GStreamer" (GNOME Bugzilla)]

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

--- Comment #10 from Sebastian Dröge (slomo) <***@coaxion.net> ---
It is sometimes necessary for compressed data outside the segment to be pushed
downstream to be able to decode the data that is inside the segment.

Also here, segment.duration is only informational. It is not uncommon to have
streams that continue after their duration (if the duration is only an estimate
for example, or headers are broken), and streams should just continue until
they're done.
segment.start and segment.stop are the parts that are not just informational
but actually enforced.

["GStreamer" (GNOME Bugzilla)]

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

--- Comment #11 from Rajesh Singh <***@gmail.com> ---
i feel, it may not be applicable in case of asfdemux, as we have following
lines of statement in asfdemux code.

if (demux->play_time > 0) {
demux->segment.duration = demux->play_time;
}

["GStreamer" (GNOME Bugzilla)]

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

--- Comment #12 from Sebastian Dröge (slomo) <***@coaxion.net> ---
Why do you think this makes a difference?

["GStreamer" (GNOME Bugzilla)]

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

--- Comment #13 from Rajesh Singh <***@gmail.com> ---
To skip a frame or for making decision, whether frame is abnormal,
I am taking the reference of "demux->segment.duration"

if(demux->segment.duration > 0
&& GST_BUFFER_PTS (payload->buf) > demux->segment.duration)){

}

["GStreamer" (GNOME Bugzilla)]

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

--- Comment #14 from Sebastian Dröge (slomo) <***@coaxion.net> ---
Yes but that's a circular argument :) The point is that segment.duration is
mostly informational and not enforced, so if play_time / segment.duration have
wrong values this doesn't matter for now. There could be files where those
fields are wrong, unless that would make the file generally unplayable for
other reasons.

["GStreamer" (GNOME Bugzilla)]

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

--- Comment #15 from Rajesh Singh <***@gmail.com> ---
Test Stream Location:
http://wikisend.com/download/164788/Test_Stream.wmv

Patch was uploaded to fix the playback issue.
Play the full video, video freezes, after few seconds.

["GStreamer" (GNOME Bugzilla)]

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

--- Comment #16 from Rajesh Singh <***@gmail.com> ---
Any progress on this issue?

["GStreamer" (GNOME Bugzilla)]

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

GStreamer system administrator <***@gstreamer.net> changed:

What |Removed |Added
----------------------------------------------------------------------------
Status|NEW |RESOLVED
Resolution|--- |OBSOLETE

--- Comment #17 from GStreamer system administrator <***@gstreamer.net> ---
-- GitLab Migration Automatic Message --

This bug has been migrated to freedesktop.org's GitLab instance and has been
closed from further activity.

You can subscribe and participate further through the new bug through this link
to our GitLab instance:
https://gitlab.freedesktop.org/gstreamer/gst-plugins-ugly/issues/12.