This evening, while making no change to my website I created a new blog post with a new embedded Soundcloud file, and found that Drupal only displayed the URL for the tune rather than the player. Older posts were still working fine, until I try to resave them.

The website is http://www.yeahmusic.co.uk/ - this is a really weird one, not sure if it is me or Soundcloud themselves.

I've updated this module to the dev version, as well as making sure all the other required modules are up to date, but no clue.

Any ideas anyone?

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

ewills’s picture

Hmm - digging into it, it looks like Soundcloud have changed their embed codes on their music pages.

I think line 113 in soundcloud.inc is the problem:

$h1tags = preg_match('/(
)/i', $response->data, $patterns);

This no longer comes up with any matches.

I'll have a go at some new pattern matching tomorrow to get it to recognise the new XML link which contains the swf link.

ewills’s picture

Category: support » bug
Status: Active » Patch (to be ported)
FileSize
1.68 KB

Ok - I've fixed it for when you just add a URL to the CCK field (rather than the whole embed code).

The module was generating the embed code itself in a bit of a hacky way. I've now enabled it to use the proper Soundcloud oEmbed method, meaning Soundcloud always generates the embed code automatically for you from the tune url.

I haven't tested this with embedding the whole embed code (it should work fine anyway as I haven't touched any of that side of things). I also haven't tested it for sets of music, but as Soundcloud is generating the code itself I can't see why it wouldn't work.

Edit:
Sets work just fine :)

joachim’s picture

Version: 6.x-1.1 » 6.x-1.x-dev
Status: Patch (to be ported) » Needs work

> The module was generating the embed code itself in a bit of a hacky way.

Yup! Thanks for taking a look at this :)

+++ providers/soundcloud.inc	12 Jan 2011 13:16:49 -0000
@@ -112,7 +105,10 @@ function emaudio_soundcloud_data($field,
+    $xml = file_get_contents($swf);

Should we use drupal_http_request here?

+++ providers/soundcloud.inc	12 Jan 2011 13:16:49 -0000
@@ -112,7 +105,10 @@ function emaudio_soundcloud_data($field,
+    $xml = new SimpleXmlElement($xml); ¶

And here, drupal_xml_parser_create()?

Powered by Dreditor.

ewills’s picture

Status: Needs work » Needs review
FileSize
1.69 KB

I've updated the patch to use the drupal_http_request, but not the drupal_xml_parser_create(), as doing this means I have to use xml_parse_into_struct(), which then creates a pretty nasty array that I'd have to loop through just to get the HTML bit. In SimpleXML it's just a case of accessing the appropriate child in the object.

Unless there's a specific reason for not using SimpleXML (or you can come up with a nicer solution using drupal_xml_parser_create()) I'd quite like to keep it as SimpleXML.

apoc1’s picture

Hey, I'm having the same problem, but I can't patch the module, as I don't have enough permissions on the server (shared hosting)

Is there any other way to fix this (even manual?)

ewills’s picture

FileSize
9.64 KB

I've attached the module with the patch applied - that should work for you.

joachim’s picture

> but I can't patch the module, as I don't have enough permissions on the server (shared hosting)

Patch the module on your own system, then upload the resulting module.

joachim’s picture

+++ providers/soundcloud.inc	12 Jan 2011 17:59:47 -0000
@@ -112,7 +105,10 @@ function emaudio_soundcloud_data($field,
+    $xml = drupal_http_request($swf);
+    $xml = new SimpleXmlElement($xml->data); ¶

Can someone test what happens if the user puts in a bad embed code? Should this part of the code check first for a correct response from the server before trying to parse the XML?

Powered by Dreditor.

ewills’s picture

If the url itself is invalid, the module doesn't let the user submit the content.

If the url is valid but doesn't go anywhere (404), the module just displays the URL in place of the soundcloud player in every instance it is called. What would you like it to do?

joachim’s picture

I'd say if the soundcloud website doesn't return something valid, then this module should throw up an error to the user if it still can from this hook.

What do other emfield modules do in this case, eg YouTube?

ewills’s picture

With YouTube, if you put in an invalid URL (i.e. http://www.youtube.com/watch?v=Luaaaa71nET) the content is allowed to be saved, the video just doesn't show up.

I'll have a look at what I can do from within the hook tomorrow.

joachim’s picture

I think as long as we do the same as the YouTube module, that's ok.

dreamdust’s picture

Patches don't work for me.

joachim’s picture

Status: Needs review » Needs work

Can you give more detail please? Did they not apply, or did something go wrong later?

dreamdust’s picture

Status: Needs work » Needs review

Dunno what details to give. Drupal 6.20, applied fine. Cleared cache. No errors. It's doing the same thing as before, it just stopped embedding and displays the URL. Is the patch working for you? I tried both patches.

ewills’s picture

Could you give an example of a link you're trying to embed?

szantog’s picture

Status: Needs review » Reviewed & tested by the community

#13 you need to SAVE the node. I patched the the module, and saw, it didn't work. I started to debug it, then figured, the field->data doesn't update self when view a node. :)
This patch works me, but starting from this I was wondering, why is the complete swf object stored in $field['delta']['data']. Build the html, embed code is theme logic, isn't it?

szantog’s picture

Temporary solution, i merged the patch in a theme function in my theme:

function THEME_NAME_emaudio_soundcloud_flash($item, $embed, $width, $height, $autoplay) {
  // If the swf element is NULL, then we have an embed code instead of a URL;
  // return this directly.
  if (is_null($item['data']['swf'])) {
    $response = drupal_http_request(EMAUDIO_SOUNDCLOUD_MAIN_URL . $item['value']);
    if ($response->code == 200) {
      $patterns = array();
      $h1tags = preg_match('/(<link href="(.*)" rel="video_src" \/>)/i', $response->data, $patterns);
      $data['player'] = isset($patterns[2]) ? explode('&', $patterns[2]) : NULL;
      $swf = 'http://soundcloud.com/oembed?url=http://soundcloud.com/'.$item['value'];
      $xml = drupal_http_request($swf);
      $xml = new SimpleXmlElement($xml->data);
      $item['data']['swf'] = (string)$xml->html;
    }
    else {
      return $item['embed'];
    }
  }

  $output = '';
  if ($embed) {
    if ($item['data']['emaudio_soundcloud_version'] >= 1) {
      $id = form_clean_id('soundcloud');
      $autoplay = $autoplay ? '&amp;auto_play=true' : '';
      $output .= $item['data']['swf'];
    }
  }

  return $output;
}
dreamdust’s picture

This patch works me, but starting from this I was wondering, why is the complete swf object stored in $field['delta']['data']. Build the html, embed code is theme logic, isn't it?

I can confirm this is the problem. I had to re-save the node. Of course, I have 300+ nodes with soundcloud embeds that I have to save now.

Tell me, why does the module store the embed code? Is it really a big deal to hit soundcloud's servers everytime someone needs an embed code?

ewills’s picture

You can quickly save all of these nodes by going to the content pages - admin/content/node - and selecting all the nodes you need saving, then choosing "Reload embedded media data" in the update options and click "Update"

joachim’s picture

> Tell me, why does the module store the embed code? Is it really a big deal to hit soundcloud's servers everytime someone needs an embed code?

Is that not standard? Again -- please compare with Youtube module.

ewills’s picture

Hmm - yeah the YouTube module just stores the YouTube id in the media_youtube_node_data field - I'll take a look at adapting this module to do the same thing.

dreamdust’s picture

Well there's no benefit to storing the embed code is there? I mean, if you're already hitting the server to load the flash widget then loading the embed code in addition to the widget isn't going to add much if any overhead.

You can quickly save all of these nodes by going to the content pages - admin/content/node - and selecting all the nodes you need saving, then choosing "Reload embedded media data" in the update options and click "Update"

Thanks! That's a big help.

joachim’s picture

> I mean, if you're already hitting the server to load the flash widget then loading the embed code in addition to the widget isn't going to add much if any overhead.

It's making two remote calls instead of one.

Best see what YouTube does and stick to that, since that was previously a part of emfield core, and so presumably does the Right Thing. Can someone look into that please?

ewills’s picture

Status: Reviewed & tested by the community » Needs review
FileSize
1.8 KB

The YouTube module does not store the embed code itself, instead it queries YouTube for it (so if the embed code changes you don't need to update all your modules).

I've fixed the Soundcloud patch so it follows the same behaviour now, where it pulls the embed code straight from Soundcloud every time a tune is displayed.

nodecode’s picture

subscribing.

jseffel’s picture

Tried the patch from #25:
patch -p0 < fix_soundcloud_player_v3.patch
patching file providers/soundcloud.inc

After that I tried to embed: http://soundcloud.com/timtainment/sets/genre-set-techno/

I got the "You have specified an invalid media URL or embed code." message.

joachim’s picture

Status: Needs review » Needs work
AntiNSA’s picture

+1

dLux’s picture

with the v3.patch i get:
Fatal error: Uncaught exception 'Exception' with message 'String could not be parsed as XML'

with no patch i get a couple:
warning: SimpleXMLElement::__construct() [simplexmlelement.--construct]
messages..

v1 & v2.patch aren't working either; +1 subscribing..

cyrilg’s picture

whenever I try to embed something either using an url or the embed code from Soundcloud, I get the following error after saving the node:

warning: Parameter 6 to emaudio_soundcloud_audio() expected to be a reference, value given in //sites/all/modules/emfield/emfield.module on line 650.

Anybody knows how to fix that ?

cyrilg’s picture

To fix it I had to modify the code of the file media_soundcloud/providers/soundcloud.inc.
On line 155 instead of
function emaudio_soundcloud_audio($embed, $width, $height, $field, $item, &$node, $autoplay)
it needs to be:
function emaudio_soundcloud_audio($embed, $width, $height, $field, $item, $node, $autoplay)
(Notice the "&" missing on the 6th parameter)

nodecode’s picture

so can anyone else confirm if fixing the typo as in #32 does the trick?

AntiNSA’s picture

I can

joachim’s picture

Just a reminder from your maintainer of what needs to happen to fix this:

1. someone makes a patch and uploads it
2. at least 2 people try the patch and confirm it fixes the original problem

joachim’s picture

I'm also confused as to how we've gone from patches earlier in this issue that changed a fair bit of code, to just removing a &...

shiroitatsu’s picture

Confused as well: i used the patches & corrected the typo, but still not working.

nodecode’s picture

I used #6 and it seemed to work just fine for me, however, according to the thoughtful conversation between ewills and joachim, this is not future-proof if soundcloud decides to change their player embed code in the future. So ewills attempted to modify providers/soundcloud.inc to query the embed code directly from soundcloud's servers in #25 with the v3 patch (Note: for anyone trying to apply the v3 patch, it's against the 6.x-1.1 branch not the 6.x-1.x-dev branch or any of the previous patches in this thread).

Results from testing the v3 patch...
I've had no problems with any soundcloud urls EXCEPT when I used the ulr from #27 (http://soundcloud.com/timtainment/sets/genre-set-techno) i got a white screen of death when viewing the node. I went to the actual page on soundcloud and found that there is no embed code for this particular audio clip when clicking [share]. I believe this may have something to do with the problem I and others are seeing (more details below)...

After seeing the white screen, if I go back to edit the node and enter any other soundcloud url, it works, however, i get this set of error messages ONLY on the first view of the updated (working) node:

  • warning: SimpleXMLElement::__construct() [simplexmlelement.--construct]: Entity: line 1: parser error : Start tag expected, '<' not found in /var/www/mysite/sites/all/modules/media_soundcloud/providers/soundcloud.inc on line 88.
  • warning: SimpleXMLElement::__construct() [simplexmlelement.--construct]: in /var/www/mysite/sites/all/modules/media_soundcloud/providers/soundcloud.inc on line 88.
  • warning: SimpleXMLElement::__construct() [simplexmlelement.--construct]: ^ in /var/www/mysite/sites/all/modules/media_soundcloud/providers/soundcloud.inc on line 88.
  • warning: SimpleXMLElement::__construct() [simplexmlelement.--construct]: Entity: line 1: parser error : Start tag expected, '<' not found in /var/www/mysite/sites/all/modules/media_soundcloud/providers/soundcloud.inc on line 88.
  • warning: SimpleXMLElement::__construct() [simplexmlelement.--construct]: in /var/www/mysite/sites/all/modules/media_soundcloud/providers/soundcloud.inc on line 88.
  • warning: SimpleXMLElement::__construct() [simplexmlelement.--construct]: ^ in /var/www/mysite/sites/all/modules/media_soundcloud/providers/soundcloud.inc on line 88.

    I reverted back to the patch in #6 and got similar results except that the error was on line 110

    • warning: SimpleXMLElement::__construct() [simplexmlelement.--construct]: Entity: line 1: parser error : Start tag expected, '<' not found in /var/www/mysite/sites/all/modules/media_soundcloud/providers/soundcloud.inc on line 110.
    • warning: SimpleXMLElement::__construct() [simplexmlelement.--construct]: in /var/www/mysite/sites/all/modules/media_soundcloud/providers/soundcloud.inc on line 110.
    • warning: SimpleXMLElement::__construct() [simplexmlelement.--construct]: ^ in /var/www/mysite/sites/all/modules/media_soundcloud/providers/soundcloud.inc on line 110.

      @joachim/ewills: Can I be of any further assistance in testing this more thoroughly?

      nodecode’s picture

      still testing... i've found no errors with any soundcloud urls other than that one from #27.

      Is there some way someone can take a look at what is being returned by:
      $xml = drupal_http_request($swf);
      ...on line 87 of the v3 patched soundcloud.inc to see what is the value of $xml when using the soundcloud url from #27?

      Perhaps then we can figure out why SimpleXmlElement is throwing those errors.

      AntiNSA’s picture

      I can confirm # 32 works. when using views I dont use the display embedded code field, I use the default. Everything works great. I think it should be marked as fixed.

      AntiNSA’s picture

      FileSize
      8.87 KB

      here is a working version with the typo fixed. It works great for me.

      nodecode’s picture

      @AntiNSA: I think you and kenzax may be discussing a different issue. I'm not entirely sure what it is that you're trying to resolve.

      The version you uploaded also adds a "+" to line 85. I don't know where you're going with this but, for me, it's not a solution to the problems described in #30 or #38.

      AntiNSA’s picture

      there may be an extra + . I will check it. But it is working great to embedd soundcloud with audio embedd... I need it to do so with fbsm. With fbsmp it is treating the link like a normal link.

      nodecode’s picture

      Backing up for a moment here... I've been testing ewills' versions of the module starting from #6 and i believe this is where the problems started.

      @ joachim, in response to #8 where you asked:

      Can someone test what happens if the user puts in a bad embed code? Should this part of the code check first for a correct response from the server before trying to parse the XML?

      I've tested thoroughly and I say YES the code SHOULD check first for a correct server response (code 200) before executing the SimpleXmlElement function because this is causing some of the errors I and other users are seeing. Not only can this version of the module not handle soundcloud server errors, it appears embed code can no longer be entered (only a simple valid url will work).

      Try it yourself:

      1. Install the module version from #6
      2. Enter a NON-EXISTENT URL: http://soundcloud.com/audiological-1/for-old-time-sak and it will not work. As ewills said in #9, instead of the player you just see the non-existent URL as text.
      3. Now try this VALID BUT BAD URL: http://soundcloud.com/timtainment/sets/genre-set-techno . SimpleXmlElement fails (i see white screen of death) because an Error 403 Forbidden code is returned.
      4. Now try this WORKING URL: http://soundcloud.com/audiological-1/for-old-time-sake and it should work fine (code 200 is returned).
      5. Now try using the EMBED CODE from the player of the WORKING URL in step 4. Again SimpleXmlElement fails (white screen of death for me), this time because an Error 404 Not found is returned.

      It all points back to lines 109-110 and the SimpleXmlElement:

      $xml = drupal_http_request($swf);
      $xml = new SimpleXmlElement($xml->data); 
      

      In between those two lines should be a check for a proper response from the server. Perhaps like:

      $xml = drupal_http_request($swf);
      if(($xml->code) == 200) { // code 200 means successful query of soundcloud servers, ok to parse resulting xml
           $xml = new SimpleXmlElement($xml->data); }
      

      I have more test results to share if any of you are interested in fixing this together.

      nodecode’s picture

      FileSize
      2.3 KB

      I made two changes to ewills' version in #25 and now all tests succeed for me in each situation I outlined in my last post.

      Changes made:

      1. Line 88: Added a check to make sure the SoundCloud servers have responded with a proper code before trying to parse any xml.
      2. Line 77: Changed the logic by which the module assumes a user entered embed code instead of a URL. It no longer checks if the $item['data']['swf'] variable is null (which was ALWAYS the case), it now checks if the user entered code with <object></object> tags (this is how soundcloud embed code is formatted). Perhaps there's a better way to do this? But it works.

      @AntiNSA: this version does not attempt to fix the issue you're seeing, though your fix should be easy to implement if it's still a problem.
      @joachim: your insights would be most helpful

      I have no idea how to create a patch, so I've uploaded soundcloud.inc. Just drop in the "media_soundcloud/providers" directory to test.

      joachim’s picture

      Everything you need to know about patches: http://drupal.org/patch

      Please have a go at making one -- it makes the job of reviewing changes so much easier.

      nodecode’s picture

      So I tried creating the patch with Git and its frustratingly complicated (i wasted 4 hours of my life). I gave up and created this patch with ssh diff command directly on my server. Hopefully this works, if not let me know. Please keep in mind it's my first attempt at creating a patch... ever!

      joachim’s picture

      4 hours!? Was it problems installing things on Windows? There's a workflow specific to this module here too: [#866584].

      At any rate, congratulations on your first Drupal patch! :D
      It looks pretty good, though it'll need a reroll with spaces instead of tabs, and some coding style issues (use Coder module to check for those).

      nodecode’s picture

      Okay i have a new patch rolled with Git. Will this work?

      nodecode’s picture

      Sorry, I forgot to clean up the code... this patch should be fit for testing. Feedback is appreciated.

      joachim’s picture

      Status: Needs work » Fixed

      Thanks!

      Finally had some time to work on this... :)

      > the YouTube module just stores the YouTube id in the media_youtube_node_data field - I'll take a look at adapting this module to do the same thing.

      I had a look at the YouTube module, and from what I can tell the reason it doesn't need to store embed code is that it expects to find local flash players which it can just give the YouTube URL to. We don't have that here, so I'm restoring the cached embed code.

      Rerolled with a few other fixes for code style.

      #1023718 by ewills, nodecode, joachim: Fixed Soundcloud embedding.

      Status: Fixed » Closed (fixed)

      Automatically closed -- issue fixed for 2 weeks with no activity.

      jseffel’s picture

      I'm still having problems. Embedding soundcloud media via embed code works fine, but using the URL does not. The URL validates but the player is missing.

      Reopen?

      joachim’s picture

      File a new bug please?