Comments

joachim’s picture

Component: Miscellaneous » Code

The project page says:

It is only basically maintained (ie, patches will be reviewed and committed). If you want to offer to co-maintain it, please file an issue here.

That means if someone posts a patch to port this to D7, I will review it and commit it. So the answer to your question is yes, if you or someone else who needs this puts in the work :)

jagermonster’s picture

StatusFileSize
new67.72 KB

I have created a port of this to d7.
find attached in zip format.

philosurfer’s picture

Any Input on this patch?

I will attempt tomorrow night.... Thanks for the hard work JagerMonster!

joachim’s picture

Big patch. Given emfield no longer exists on D7, what does this work with?

pau1_m’s picture

@philosurfer It's not a patch, it's a fully fledged module.

@joachim Emfield has become the Media module. It seems to be standard to have 'Media: Example' 6.x work with Emfield and 7.x work with Media. e.g. http://drupal.org/project/media_youtube

@jagermonster Brilliant! Tried it out and it works. Getting this committed to a 7.x branch will make lots of people happy (including me).

Thanks :)

Anonymous’s picture

@jagermonster Thanks so much! I was just about to start on this myself and was kind of dreading it, so your work is greatly appreciated.

philosurfer’s picture

Can we get this committed?

joachim’s picture

Status: Active » Needs work

Just had another look at this -- it's a bit of a monster.

Why do we now have CSS and JS where we had no need before?

Could you explain patterns like this:

> * Wrapper for variable_get() using the Media: SoundCloud variable registry.

Are they to do with how Media module works things, or just something you've added in?

I would like to see a minimum of new invention for a core version change -- and I certainly can't commit something this complex which has large portions I don't understand.

drupa11y’s picture

Genious !!! Thanks !!!

jagermonster’s picture

For this module i used the media_youtube module as example. the reason the variable_get function gets replaced by a custom one is to avoid conflicts with other media modules.

e.g.
'default settings' => array(
'width' => media_soundcloud_variable_get('width'),
'autoplay' => media_soundcloud_variable_get('autoplay'),
),
now this is because youtube and flickr and other media modules might use similar values. This way there is no problem because i prefix the variables with "media_soundcloud__"

joachim’s picture

So why not:

'default settings' => array(
  'width' => variable_get('media_soundcloud_width'),
  'autoplay' => variable_get('media_soundcloud_autoplay'),
),

> For this module i used the media_youtube module as example

Yup, but I don't know how that code works and therefore I can't maintain it. I'm sorry, but this needs to be dramatically scaled back.

jagermonster’s picture

#11 all it does is add a prefix, there is no reason these functions cant be removed and replaced by just normal variable_get functions, other than manually having to prefix variable names.
might be worth asking on the media_youtube thread if there is any specific reason they chose to do it that way.

One thing though on the page admin/config/media/file-styles/edit/#file_style_name#(#file_style_name# is either square_thumbnail,thumbnail,large,etc...) there is a Preview section that shows a preview to the user. This use the values set in media_soundcloud_variable_default to create this preview

joachim’s picture

> This use the values set in media_soundcloud_variable_default to create this preview

Are you saying Media module invents a hook_variable_default()? If so, urgh.

jagermonster’s picture

You can make me a co-maintaner for this module if you would like. i have had to do a few other media modules, including media_flickr, media_vimeo and this media_soundcloud for a project that im working on.

The media module has been updated to 7.x-1.0-beta5
Last test was done on beta4 so there might be some changes since the update.

Anonymous’s picture

You'll need to apply the fix from this media_youtube patch: http://drupal.org/node/1203252#comment-4668468

jagermonster’s picture

StatusFileSize
new67.73 KB

Applied patch, now compatible with latest version of media module 7.x-1.0-beta5

Find attached zip file of the latest version of this module.

jagermonster’s picture

StatusFileSize
new67.74 KB

Bug in previous zip file dont use it. use this one

jagermonster’s picture

StatusFileSize
new67.76 KB
new67.76 KB

Gaw, wrong file again !! sorry. here is the correct one with patches
function media_soundcloud_formatter_filter($variables) {
if (isset($variables['object'])) {
$object = isset($variables['object']->file) ? $variables['object']->file : $variables['object'];
return (file_uri_scheme($object->uri) == 'soundcloud') && ($object->filemime == 'audio/soundcloud');
}
}

/**
* Implementation of the File Styles module's hook_file_styles_filter().
*/
function media_soundcloud_file_styles_filter($object) {
$file = isset($object->file) ? $object->file : $object;
if ((file_uri_scheme($file->uri) == 'soundcloud') && ($file->filemime == 'audio/soundcloud')) {
return 'media_soundcloud';
}
}

dddave’s picture

Just for the record: This should be built against the new 7.x-2.x branch...

joachim’s picture

@jagermonster I really should make you a co-maintainer, as I don't have time to work on this and I was only ever a very occasional maintainer here. I could commit the latest work as the start of a D7 branch, but I think it will need more work.

Are you at Drupalcon so we can talk about it?

jagermonster’s picture

soz i didnt see this message in time, as i was at drupalcon last week.

ZuluWarrior’s picture

Sub,

Great efforts guys, thank god I found this module in the issue queue!!!

+1 for getting this set up under a 7.x branch!

joachim’s picture

@jagermonster: Heh, that's where I was too! ;) I guess we'll have to chat on IRC sometime instead?

Argus’s picture

Tested #18 and that seems to work fine. It follows all the conventions the media module uses. This is purely from an end-users point of view. If I run into any problems I will report it, but when installing and configuring works fine then that justifies a 7.x-dev version right? :)

Thanks!

Argus’s picture

Since there is no other place to file an issue on the D7 version then this one I will report it here, as said above #18 worked fine, but tonight I got the following error:

Error Embeding SoundCloud Media code(-110) :
Please check sharing permissions

for each embedded Soundcloud file. Notice the spelling error "Embeding" instead of Embedding. I think this error was caused by the Soundcloud server not being available and it is a bit confusing.

It should read: "Soundcloud server not available" or something similar.

philosurfer’s picture

Soundcloud was down all day yesterday.

joachim’s picture

Status: Needs work » Fixed

I've committed the most recent zipfile from #18:

- #1022266 by jagermonster: Added Drupal 7 version of the module.

The dev release will be available later today.

I remain sceptical about the large increase in complexity in this module, and I'm not going to be able to actively maintain the new 7.x-1.x branch. That means that it's up to users of this branch to write patches and review them. I will commit patches that go through that process and get set to RTBC.

Anyone willing to co-maintain should file an issue with links to either projects they maintain or patches they've had accepted to other projects by way of references.

joachim’s picture

Version: 6.x-1.x-dev » 7.x-1.x-dev
Argus’s picture

Title: D7 port » D7 port for Media: SoundCloud

thanks!

Status: Fixed » Closed (fixed)

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

victoriachan’s picture

Great work, everyone!

Is it possible to create a recommended release for this? I know it's not really stable yet, but we could just give the tag a suffix of 'beta' or 'unstable'.

I'm using drush makefile for my project. While the 7.x-1.x-dev code works for me the last time I downloaded it, I can't be sure that it won't be changed the next time I deploy the site.

I could just use the md5 hash of the download in my makefile, but it would be more useful for other people too if they can choose to use a 'frozen' version of the it, rather than the dev branch which could have new commits every now and then.

Thanks,
Victoria

joachim’s picture

Good point. Really bugs me when maintainers leave new bugfix code sitting in dev where most people can't or won't use it. I've made an alpha 1 release for 7 and a new 6 release since there were a few bug fixes that are quite old now :/

Thanks for the gentle reminder :)

victoriachan’s picture

Thank you!

rob c’s picture

About the:

Error Embeding SoundCloud Media code(-110) :
Please check sharing permissions

Maybe a rename to indeed something more general like server down and then maybe a watchdog or something with more details, visitors dont really want to know about error codes, they want music :)

And if i for example have a list of songs in a view, i get this message for every song in that view. Inline on the spot where the player should be makes more sense i think. (or at least a config option for this)

And a lil bit more on styles, would love more features on that part, but for everything it should do + the html5 player patch, awesome work people!

joachim’s picture

Please file new issues for those, thanks :)

rabellamy’s picture

Category: feature » task
Priority: Normal » Major
Status: Closed (fixed) » Needs review
StatusFileSize
new934 bytes

I've edited the spelling on the $error_msg.

joachim’s picture

Category: task » feature
Status: Needs review » Closed (fixed)

Please file new issues for those, thanks :)

victoriachan’s picture

Status: Closed (fixed) » Fixed

Status: Fixed » Closed (fixed)

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