2520 replies to this topic

[christophecvr]

Quick feedback for todays build from my beta tester.

"It's working EXCELLENT"

Tested on DM8000.

 

This is with:

- OpenPLi from gst-1

- gstreamer1.0-plugin-multibox-dvbmediasink from Christophecvr

- mx3L's pull request from here: https://github.com/O...nigma2/pull/100

- Gstreamer master git

- librtmp1 and rtmpdump from oe-alliance

 

Even some HLS streams are playing now :-)

 

A big shout-out to everyone who helped out submitting fixes and bugreports.

Especially Athoik, Chris and mx3L!

 

I'm now building beta-images for everyone (I need a faster build server).

 

I hope to have them ready and posted on friday.

Yes all true. But unfortunately gstreamer version 1.5.90 or above does has a very general regression for variable bitrate mpeg media. I'm now bussy triyng to find out which commit causes the regression for us on gstreamer. Will take  lot of time.

[christophecvr]

@MastaG

 

Note: looks again that pli4 team really does not wan't to improve anything about pause unpause behaviour. Again a very very extreme and needed pull request is not merged. ????

[littlesat]

That is not true.... we need to better understand why this might work... as far I know there is a hardware buffer behind... So doing sync stuff in gstreamer should not have any effect on this behavior....

 

We just need a better clarification/description!

[mx3L]

That is not true.... we need to better understand why this might work... as far I know there is a hardware buffer behind... So doing sync stuff in gstreamer should not have any effect on this behavior....

 

We just need a better clarification/description!

Hi,

 

Which patch needs more clarification?

dvbaudiosink patch: https://github.com/O...ediasink/pull/5

,servicemp3 patch: https://github.com/O...nigma2/pull/100

or both?

 

Is there anything specific what's not clear or most of it?

 

I tried to do some explanation in here:

http://forums.openpl...e-improvements/.

 

But I can try to clarify it even more if neccessary.

Edited by mx3L, 19 November 2015 - 08:21.

[MastaG]

That's why I've applied it manually myself.

I figured out you can just put ".patch" behind each commit.

Like this: https://github.com/m...37909f1b9.patch

I can simply remove it from my bbappend once it's merged.

 

So it's already checked into my beta images.

 

However (unrelated to gstreamer) it seems we also need Taapats pull request as well.

As it fixes some issue with the GUI: https://github.com/O...enigma2/pull/98

[littlesat]

What I do not understand is that you "play" with (a)sync in gstreamer while in fact "behind" gestreamer the hardware buffer (and driver) guaranteed messed up with the "buffer".... So I do not understand how this could fix the pauze performance....???? How does it work-a-round that hardware (driver) buffer???

 

So I'm "afraid" for one type of box it could have a good effect, while for a different box it could even give a more negative effect....

 

It could also be I really do not understand it... 

 

At the end I need more info as I do not merge is when I do not understand the "background"....

 

Here: http://forums.openpl...e-improvements/ is see the issue described and understood... but I still do not understand why the latest commit suggestion should solve it.... (it might be I miss the clue).

Edited by littlesat, 19 November 2015 - 11:02.

[littlesat]

In addition I discovered in the subsink I can not merge (I think I have not enough rights on github)... In enigma 2 I can merge... but I think they need both to be merged...

 

Of course we can merge them and when later on negative side effects (that is my "scare") occur we can still revert it... but still I need better understand why disabling e.g. async does help here... The risk of merging is low as a gstreamer > 1.5 image is not really "public"...

[Beeker]

@christophecvr @athoik

 

According to my research. i found the commit that causes freezes on test.mpg.

 

http://cgit.freedesk...dad3e64f14de00a

 

With previous commit of gstreamer(866f9ace5cbf245b67ec993ef69302f2ff71595   adapter: Add get variants of the buffer based take functions
) video file plays fine. After updating gstreamer just with this commit, the video freezes.

 

This are  my githashes

 

gstreamer SRCREV = "c3bcbadd5452d5b3450f70e49dad3e64f14de00a"        is       1.5.2 plus commits till 30-6-2015.
libav SRCREV = "afdbe664602f42208948296024e9484c522f01f9"                   is        1.5.2
plugins-bad SRCREV = "ad827597e38201c6f97973c76d6131129883f574"     is         1.5.2
plugins-base SRCREV = "14083178b8c837a217825637cfcbb122e17e5d5a"  is        1.5.2  plus commits till 15-7-2015 git( improve video log). jus for test.
plugins-good SRCREV = "f12899878ffc8e4526cd01ea4b9a42b70766297c"    is         1.5.2
plugins-ugly SRCREV = "73d91f9409e36a6dd50bffe1451f216d878d062a"     is          1.5.2

Edited by Beeker, 19 November 2015 - 14:47.

[mx3L]

 What I do not understand is that you "play" with (a)sync in gstreamer while in fact "behind" gestreamer the hardware buffer (and driver) guaranteed messed up with the "buffer".... So I do not understand how this could fix the pauze performance....???? How does it work-a-round that hardware (driver) buffer???

There is no workaround hardware (driver) buffer. When doing unpause in servicemp3 we are doing flush seek to current position.

flush seek to current position:

 1. discards buffers in pipeline + discards buffers in driver buffer

 2. source is set to current position, new buffers starts to flow through pipeline into sink and driver buffer.

 

I suggest to not to do flush seek to current position when unpause, but only unpause pipeline. There is no need to discard valid buffers, we just need to continue to push buffers to driver buffer after unpause.

 

With this said it should be now clear why it improves unpause performance, i.e we don't have to re-fill pipeline after flush seek with new buffers, but seamlessly continue in playback.
This is done in servicemp3 patch + fallback to flush seek to current position, in case pipeline is not in valid state or unpause is not used on supported sources.

 

but I still do not understand why the latest commit suggestion should solve it.... (it might be I miss the clue)

When we pause pipeline, sometimes happens that it's afterwards not in valid state. We cannot unpause pipeline from not valid state.
Not valid state means, that it's not in PAUSED state and has pending state set to PAUSED, which needs to be commited first, in order for state to be valid.

 

State of pipeline is defined by it's elements. I found out that problematic element which causes this state is dvbaudiosink. dvbaudiosink is in this state, because it waits for preroll buffer, which never comes.

By turning off async state changes in dvbaudiosink, we make sure that it will never get into this state, because it will no longer care when this preroll buffer comes, but will commit new state immediately. -> pipeline should be always in valid state when unpausing, i.e. fallback option to flush seek to current position, will be not used.

 

http://gstreamer.fre...t-async-enabled

Configures sink to perform all state changes asynchronously. When async is disabled, the sink will immediately go to PAUSED instead of waiting for a preroll buffer. This feature is useful if the sink does not synchronize against the clock or when it is dealing with sparse streams.

 

Summary:
servicemp3 patch:

 

should be enough, but when unpausing it sometimes fallbacks to flush seek to current position, when pipeline is not in valid state.

If this happens on Vu+ it sometimes triggers some kind of bug, which causes to stop video rendering and only audio can be heared, while A/V buffers are flowing to sinks but are far apart(pts).

async patch:

 

makes sure that we have pipeline always in valid state after pause, so no fallback to flush seek to current position happens -> always seamless unpause, assumed Vu+ bug in drivers is not triggered.

 

Patches were tested on et4x00, vusolose and vuduo2(@christophecvr)

Edited by mx3L, 19 November 2015 - 14:49.

[littlesat]

As I can currently not merge a commit in the mediasink Eric needs to push these commits...

 

Sorry for all the discussions.... it is now clear to me...!

[Erik Slagter]

I have one question left. Does this workaround shortcomings in the drivers (mediasink) of certain manufacterers or does this benefit all models in general, by principle? If the latter, I wonder why DMM didn't implement it themselves. Maybe their drivers have shortcomings that prevent it, very well possible. Any idea?

[Erik Slagter]

To answer the second question myself: http://git.opendream...4093a0eb2247c30

 

That does indeed look like a workaround for a bug in a DMM driver.

[Beeker]

Ok backt to git master head.

 

As workaround/test

diff --git a/libs/gst/base/gstbaseparse.c b/libs/gst/base/gstbaseparse.c
index eee3c92..d49aad6 100644
--- a/libs/gst/base/gstbaseparse.c
+++ b/libs/gst/base/gstbaseparse.c
@@ -2813,6 +2813,7 @@ gst_base_parse_chain (GstPad * pad, GstObject * parent, GstBuffer * buffer)
   GstBuffer *tmpbuf = NULL;
   guint fsize = 1;
   gint skip = -1;
+  const guint8 *data;
   guint min_size, av;
   GstClockTime pts, dts;
 
@@ -3012,7 +3011,11 @@ gst_base_parse_chain (GstPad * pad, GstObject * parent, GstBuffer * buffer)
       parse->priv->next_dts = pts;
 
     /* always pass all available data */
+    data = gst_adapter_map (parse->priv->adapter, av);
+    /* arrange for actual data to be copied if subclass tries to,
+     * since what is passed is tied to the adapter */
+    tmpbuf = gst_buffer_new_wrapped_full (GST_MEMORY_FLAG_READONLY |
+        GST_MEMORY_FLAG_NO_SHARE, (gpointer) data, av, 0, av, NULL, NULL);
-    tmpbuf = gst_adapter_get_buffer (parse->priv->adapter, av);
 
     /* already inform subclass what timestamps we have planned,
      * at least if provided by time-based upstream */
@@ -3029,6 +3024,9 @@ gst_base_parse_chain (GstPad * pad, GstObject * parent, GstBuffer * buffer)
     ret = gst_base_parse_handle_buffer (parse, tmpbuf, &skip, &flush);
     tmpbuf = NULL;
 
+    /* probably already implicitly unmapped due to adapter operation,
+     * but for good measure ... */
+    gst_adapter_unmap (parse->priv->adapter);
     if (ret != GST_FLOW_OK && ret != GST_FLOW_NOT_LINKED) {
       goto done;
     }

i made a patch wich revert the commit (see post #1987).

 

File(test.mpg) play now without freezes in latest gstreamer.

 

Now look for a proper solution in patch, dvbsink, git gstreamer ?

Attached Files

•  0001-revert-use-new-gst-adapter-get-buffer.patch   1.5KB   26 downloads

[Erik Slagter]

Applied both patches.

 

If it breaks, we will know soon enough.

[dreamboxco]

how can i update the files at the dream

[MastaG]

Thank you Erik and Beeker :-)

[athoik]

@christophecvr @athoik
 
According to my research. i found the commit that causes freezes on test.mpg.
 
http://cgit.freedesk...dad3e64f14de00a
 
With previous commit of gstreamer(866f9ace5cbf245b67ec993ef69302f2ff71595   adapter: Add get variants of the buffer based take functions
) video file plays fine. After updating gstreamer just with this commit, the video freezes.

Well done @Beeker!

I'll check soon if reverting that commit fixes the issue also for dm800se (currenlty my playground machine).

[littlesat]

Thanks Eric... and sorry from my side for all the additional questions....

[Erik Slagter]

Littlesat, we are the ones signing off, we should definitely know what we're signing, I think it's healty practise. If not only for ourselves, also for other developers.

[athoik]

@christophecvr @athoik
 
According to my research. i found the commit that causes freezes on test.mpg.
 
http://cgit.freedesk...dad3e64f14de00a
 
With previous commit of gstreamer(866f9ace5cbf245b67ec993ef69302f2ff71595   adapter: Add get variants of the buffer based take functions
) video file plays fine. After updating gstreamer just with this commit, the video freezes.

Well done @Beeker!

I'll check soon if reverting that commit fixes the issue also for dm800se (currenlty my playground machine).

@Beeker, everything is fine without that commit. GStreamer ticket updated: https://bugzilla.gno...g.cgi?id=758234