Created attachment 299685 [details] [review]
Proposed patct to parse proxy username/password from http_proxy

Created attachment 299686 [details] [review]
Proposed patct to parse proxy username/password from http_proxy

I'd rather like to see an interactive callback then another custom implement of http_proxy (the lower case version is even worst to my eyes).

Review of attachment 299685 [details] [review]:

This is no a good idea. Automatic proxying should come from system setting. If you system proxy configuration does not support http_proxy, you should request there. What is an acceptable solution would be to add callbacks, so you can have an interactive authentication. This is supported by libsoup.

Review of attachment 299686 [details] [review]:

Same.

Hello Nicolas,

I am testing this on custom platform. My point was, as we already use http_proxy variable for setting proxy url inside souphttpsrc, so why not just set proxy username/password also from there, if they are available?

Little more detail about problem, pipeline I am trying to play is something like below:

gst-launch-1.0 playbin uri=http://www-itec.uni-klu.ac.at/ftp/datasets/mmsys13/redbull_4sec.mpd

Actually, we have our own application, which creates the pipeline, allowing us to set properties, play-plause etc.

In that, I tried to register this callback:
  g_signal_connect (player->elements.playbin, "source-setup",
      G_CALLBACK (player_cb_source_setup), player_p);

and inside it, set proxy-id,proxy-pw.

With this, manifest is downloaded correctly, however, when dashdemux tries to download the first segment, I see "407, proxy authentication required" error.

Inside, dashdemux, I see, when it goes to fetch a segment, it creates an instance of souphttpsrc using below api:
gst_element_make_from_uri()

Now, with http_proxy set in my environment, this newly created souphttpsrc instance correctly goes to proxy, but this instance does not has proxy-id, proxy-pw set, so segment fetching fails.

Note that dashdemux will do this for every segment it wants to fetch.

May you please guide me on how to correctly set up proxy-id/proxy-pw on these intermediate souphttpsrc instances, if parsing http_proxy is not a viable approach?

I must say I didn't notice we had support for that. I think it is a bad idea in general. Proxy configuration is something that depends on the platform, and the http_proxy is very ill defined and implemented differently all over the place. Clearly souphttpsrc is no exception.

What I think would be useful, is to allow application to react interactively when authentication is needed. In this case, SoupSession has the "authenticate" signal. This can be called for normal HTTP authentication or for proxy (see soup_auth_is_for_proxy()). As there is no way to access SoupSession safely, I think that souphttpsrc element should provide a callback to relay. One could then properly interact with authentication and avoid using deprecated forms of user/passwd passing (http://user:pass@...).

Just for sake of clarification, what I undertand is currently, souphttpsrc connects with "authenticate" signal provided by libsoup:
  g_signal_connect (src->session, "authenticate",
      G_CALLBACK (gst_soup_http_src_authenticate_cb), src);

Inside, this callback, souphttpsrc tried to authenticate using its proxy-id & proxy-pw using below code:
static void
gst_soup_http_src_authenticate_cb (SoupSession * session, SoupMessage * msg,
    SoupAuth * auth, gboolean retrying, GstSoupHTTPSrc * src)
{
....
      if (src->proxy_id && src->proxy_pw)
        soup_auth_authenticate (auth, src->proxy_id, src->proxy_pw);
....
}

So, what you are implying is to implement a signal in souphttpsrc itself, which application/client can connect to, right?

If this is the case, can you please provide some snippet about when that signal should be emitted by souphttpsrc as we would already be inside gst_soup_http_src_authenticate_cb() callback when we become aware that proxy credentials are required?

Has to be sync message I think. And yes, has to be within the callback, unless soup as an async method to handle it.

(In reply to Nicolas Dufresne (stormer) from comment #10)
> Has to be sync message I think. And yes, has to be within the callback,
> unless soup as an async method to handle it.

Hello Nicolas,

I am still not sure I get your point.
Can you please provide some code snippet about how sync message should be posted by souphttpsrc?

Hello Nicolas,

One more update from my side:

Just checked code of 
- gst-plugins-bad/gst-libs/gst/adaptivedemux/gstadaptivedemux.c
- gst-plugins-bad/gst-libs/gst/uridownloader/uridownloader.c
- gstreamer/gst/gsturi.c

Here, we can see that for playing a MPEG-DASH stream, a new instance of souphttpsrc is created by dashdemux for each of the streams exposed in manifest file.

This souphttpsrc instance is created using api gst_element_make_from_uri().

Now the problem is we don't see any where this internal souphttpsrc instance being added to pipeline. This would pose a problem in way you are suggesting to fix it (adding sync-message...) as this message approach would only be valid for elements present in pipeline.

Please feel free if you got any suggestions.
Otherwise, I don't see a way to properly set proxy credentials on this internal souphttpsrc instance without using http_proxy.

I don't know whether I have been able to present the problem in clear way, so here is another go at it (my last one):

Inside gstadaptivedemux.c, below code is present:

gst_adaptive_demux_stream_update_source (...)
...
stream->src = gst_element_make_from_uri (GST_URI_SRC, uri, NULL, NULL);
...
gst_bin_add (GST_BIN_CAST (demux), stream->src);


This implies, say when playing a dash stream using http, containing one audio and one video track, it will create one souphttpsrc instance for each of stream.
Now these internal souphttpsrc instances are not added to pipeline demux is part of, but rather added to internal pipeline of dashdemux, via below line:

gst_bin_add (GST_BIN_CAST (demux), stream->src);

Issue I have is how to set proxy credentials (proxy_id, proxy_pw) on these internal souphttpsrc instances?

For solving this issue, I have already suggested a patch, which uses already available soup APIs to parse username/password from http_proxy variable.

If this is not acceptable, kindly suggest approach which works. But kindly don't leave this in a limbo...

The problem I see, is that you seem to assume that the proxy credential will always be the same for all URI found in the playlist. Like browsers, which should be able to handle multiple credential for multiple URI.

What I'm proprosing is to introduce a bus message for authentication. SoupSession will emit "authenticate" signal. This signal can be for normal authentication or proxy (this way we can add support for both). As we want this to happen asynchronously, we'll call soup_session_pause_message() and g_object_ref() the message). We will post a message, probably implement an Authetication inteface for GStreamer element. Application watching this message can then reply to it through the interface, and then streaming can continue.

This is not a limbo, this is what I think is the right way to solve this issue. The http_proxy environment approach is very hacky, and it's completly inconsistent between softwares. It also prevent from having certain URI go through proxy while keeping other direct.

Thanks for your replay Nicolas, now I get your point.
I am bit slow, so now when you explained in bit more detail (comment 14 vs comment 10), it became clear to me :)

Hello Nicolas,

One update, in comment 4, you mentioned
>>Automatic proxying should come from system setting.

by this, did you meant gsettings?

I tried setting http proxy username and password using gsettings but that didn't worked also, same 407 error.

Created attachment 301612 [details] [review]
souphttpsrc: use proxy username/password from http_proxy

-- 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-good/issues/170.