Switch BaseSrc to AppSrc

[Manishearth]

I'm not sure which one we plan to choose so both are still implemented here and not cleaned up too much.

r? @ferjm

cc @sdroege

[sdroege]

One general comment: with appsrc you have less control over e.g. format (caps) negotiation. You set a single caps and then produce that, there's no easy way to check what downstream wants, reconfigure when downstream changes, etc.

So if you need that, for example to always produce the format that downstream can handle and downstream can change, then this might be suboptimal. It depends on what you need, and where/who is defining the format that is to be produced

[sdroege]

These two properties are unnecessary with the way how you fill the appsrc. None of this will ever happen: your callback below is only called when data is needed. These properties only make sense if the filling of the appsrc happens driven by something else, e.g. another thread that just pushes buffers into the appsrc

[sdroege]

You probably want to handle errors here

[Manishearth]

is there a way to put the element in an error state?

[Manishearth]

I'm not sure what the model for handling errors should be here

[sdroege]

You'd check the return value of pushing but appsrc should already convert errors into an error message on the bus. You would then handle those error messages
But if an error is returned from pushing, you might want to stop processing for example

If your code itself produces an error, you could post an error message yourself. Check the appsrc example in gstreamer-rs, iirc it does that

I'll put a link to code here in a few hours when I'm back home

[Manishearth]

well, we already stop processing here, since this is the last action in the callback.

[sdroege]

Right, but a future version of this code might do more things :) Anyway, ignore this comment for the time being then

[philn]

I don't really see the benefit of using appsrc for this :)

[Manishearth]

Yeah, it was mostly something Fernando wanted to try out.

[ferjm]

Yeah, it was mostly something Fernando wanted to try out.

Thank you Manish. I wanted to look into using appsrc mostly because @sdroege suggested it, mentioning that it would make our lifes easier :). And because this is what WebKit uses for their WebAudio backend. So it seemed like something worth investigating.

I'm afraid that I couldn't look into the details and the differences of basesrc vs appsrc before my leave, so I am not in a good position to make the call here right now :. I'll have to defer to what you learned in the process of writing this patch and to @sdroege's and @philn's expertise.

I'm back from my leave on Monday, so we can chat about this in person.

[Manishearth]

Yeah, I'll just wait till next week. Is there anything you think I should look into next while I wait? -Manish Goregaokar

…

On Thu, May 10, 2018 at 3:05 PM Fernando Jiménez Moreno < ***@***.***> wrote: Yeah, it was mostly something Fernando wanted to try out. Thank you Manish. I wanted to look into using appsrc mostly because @sdroege <https://github.com/sdroege> suggested it, mentioning that it would make our lives easier :). And because this is what WebKit uses <https://trac.webkit.org/browser/webkit/trunk/Source/WebCore/platform/audio/gstreamer/WebKitWebAudioSourceGStreamer.cpp> for their WebAudio backend. So it seemed like something worth investigating. I'm afraid that I couldn't look into the details and the differences of basesrc vs appsrc before my leave, so I am not in a good position to make the call here right now :. I'll have to defer to what you learned in the process of writing this patch and to @sdroege <https://github.com/sdroege>'s and @philn <https://github.com/philn>'s expertise. I'm back from my leave on Monday, so we can chat about this in person. — You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub <https://github.com/ferjm/media/pull/4#issuecomment-388200250>, or mute the thread <https://github.com/notifications/unsubscribe-auth/ABivSOXoAlTHE29iGNey9tGlhplvoPaNks5txLmegaJpZM4T5Hvq> .

[ferjm]

It would be wonderful if you could start the implementation of the NodeEngine abstraction that you suggested and/or the mechanism to feed servo-media with them from script.

[Manishearth]

I feel like we need to discuss this further first, but I can write stuff down to prepare for the work week where we can make progress on it.

…

On Thu, May 10, 2018 at 3:55 PM Fernando Jiménez Moreno < ***@***.***> wrote: It would be wonderful if you could start the implementation of the NodeEngine abstraction that you suggested and/or the mechanism to feed servo-media with them from script. — You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub <https://github.com/ferjm/media/pull/4#issuecomment-388209950>, or mute the thread <https://github.com/notifications/unsubscribe-auth/ABivSCt0cytSVURZEGy8nj6_aeHklvDrks5txMVQgaJpZM4T5Hvq> .

[sdroege]

I wanted to look into using appsrc mostly because @sdroege suggested it, mentioning that it would make our lives easier :).

It depends on what your exact requirements are. The main possible disadvantage for this use-case is that dynamic reconfiguration of the caps (i.e. the raw audio format) based on what downstream wants is not easily possible. If you don't need that (e.g. have a fixed format, only need to decide based on downstream once, or changes are always triggered by your code) then appsrc probably makes your life easier. As seen with this PR, much less code for the same behaviour.

[ferjm]

Thank you Sebastian. I think we don't need dynamic reconfiguration of the caps.

[sdroege]

If you want, we can all together go through your requirements next week when you're back :) But for starters going with appsrc should be fine, it can still easily be changed at a later time. Just means transplanting the sample creation code into slightly different code.