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 767492 - avfassetsrc: Escapes already escaped URIs wrongly, fails to work because of that
avfassetsrc: Escapes already escaped URIs wrongly, fails to work because of that
Status: RESOLVED FIXED
Product: GStreamer
Classification: Platform
Component: gst-plugins-bad
1.9.1
Other Mac OS
: Normal normal
: 1.8.3
Assigned To: GStreamer Maintainers
GStreamer Maintainers
Depends on:
Blocks:
 
 
Reported: 2016-06-10 11:32 UTC by Sergei Saveliev
Modified: 2016-08-16 15:49 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
Rechecking uri patch (875 bytes, patch)
2016-08-16 12:06 UTC, Sergei Saveliev
none Details | Review

Description Sergei Saveliev 2016-06-10 11:32:54 UTC
in function gst_avf_asset_src_uri_set_uri used function g_uri_escape_string (uri, ":/", TRUE);
After that checking for asset.playable is always false, because [AVAsset assetWithURL: url] using percent encoded url
I'll checked out this when using GStreamer with Qt 5.6, building iOS application.

For example if I make following transformations everything is ok:
            
NSString *str=[NSString stringWithUTF8String: qPrintable(QUrl::fromNSURL(url).toString())];
NSURL *url2=[[NSURL alloc] initWithString: str];
AVAsset *ass = [AVAsset assetWithURL:url2];

In that way qPrintable returns const char* value
for example ipod-library://item/item.m4a?id=645236843028624300
Comment 1 Tim-Philipp Müller 2016-06-10 14:34:43 UTC
> in function gst_avf_asset_src_uri_set_uri used function g_uri_escape_string
> (uri, ":/", TRUE);
> After that checking for asset.playable is always false, because [AVAsset
> assetWithURL: url] using percent encoded url

What is the exact "percent encoded url" here?
Comment 2 Sergei Saveliev 2016-06-10 14:43:14 UTC
(In reply to Tim-Philipp Müller from comment #1)
> > in function gst_avf_asset_src_uri_set_uri used function g_uri_escape_string
> > (uri, ":/", TRUE);
> > After that checking for asset.playable is always false, because [AVAsset
> > assetWithURL: url] using percent encoded url
> 
> What is the exact "percent encoded url" here?

Not "percent encoded" possibly, but "escaped"
For example:

URL before when asset.playable==true 

ipod-library://item/item.m4a?id=3143338395173862951

and URL after when asset.playable==false

ipod-library://item/item.m4a%3Fid%3D3143338395173862951
Comment 3 Tim-Philipp Müller 2016-06-10 14:49:22 UTC
Ok, thanks. I would say something is wrong with the escaping then from the look of it.
Comment 4 Sergei Saveliev 2016-06-10 14:53:50 UTC
(In reply to Tim-Philipp Müller from comment #3)
> Ok, thanks. I would say something is wrong with the escaping then from the
> look of it.

Not at all! It seems to me removing tranformation with g_uri_escape_string function can fix this bug. Just put URL as is to [NSString stringWithUTF8String: url]
Comment 5 Sebastian Dröge (slomo) 2016-06-10 14:59:54 UTC
The problem is that it escapes too much, not according to the URI rules.

Not sure why it escapes anything at all though, it should assume that the caller already gives valid URIs.
Comment 6 Tim-Philipp Müller 2016-06-10 15:00:20 UTC
Yes, I mean this should not have been escaped :)
Comment 7 Sebastian Dröge (slomo) 2016-06-13 06:22:16 UTC
commit a913a0b9679dd58945ad105d240db45595fdaba6
Author: Sebastian Dröge <sebastian@centricular.com>
Date:   Mon Jun 13 09:20:22 2016 +0300

    avfassetsrc: Don't escape the URI before passing it to NSURL
    
    The URI must already be escaped by the caller, we don't support passing around
    invalid (unescaped) URIs via the GstURIHandler interface.
    
    Also it will escape too much of the URI in this case, e.g.
      ipod-library://item/item.m4a?id=3143338395173862951
    becomes
      ipod-library://item/item.m4a%3Fid%3D3143338395173862951
    
    https://bugzilla.gnome.org/show_bug.cgi?id=767492
Comment 8 Sergei Saveliev 2016-08-16 10:21:35 UTC
Very funny thing! You are fix this bug in first occurance... But what about another one?) 

Look at piece of code at lines 908-912) The same thing!
Comment 9 Sergei Saveliev 2016-08-16 10:31:13 UTC
Well and one more thing... It seems to me that this is double checking... We really need this?
Comment 10 Sebastian Dröge (slomo) 2016-08-16 11:34:15 UTC
No, if it's checking / doing the same thing twice, do it only once :)
Comment 11 Sebastian Dröge (slomo) 2016-08-16 11:34:27 UTC
Will you provide a patch for this?
Comment 12 Sergei Saveliev 2016-08-16 12:06:09 UTC
Created attachment 333405 [details] [review]
Rechecking uri patch

Here is the patch (NOT TESTED!) I have no ability to rebuild SDK for iOS for testing(
Hopes it helps!
Comment 13 Sebastian Dröge (slomo) 2016-08-16 15:49:21 UTC
Thanks, I merged the part about the escaping:

commit fc6b17b6d28c2fd52fbcaf61af0e7ba4c1db2c77
Author: Sergei Saveliev <saveliev.sergei@gmail.com>
Date:   Tue Aug 16 18:46:49 2016 +0300

    avfassetsrc: Don't escape the URI another time in another location too
    
    One location was forgotten in a913a0b9679dd58945ad105d240db45595fdaba6
    
    https://bugzilla.gnome.org/show_bug.cgi?id=767492



The part about checking playability another time I kept in there for now. The playability is currently only checked when setting the URI via the GstURIHandler interface. If it is set via the property, no checking is done at all (probably because setting properties can't fail and can't return an useful error).