Rockbox

Tasklist

FS#4899 - iPod AutoPause

Attached to Project: Rockbox
Opened by Max Ried (bot47) - Thursday, 23 March 2006, 19:03 GMT
Last edited by Rani Hod (RaeNye) - Monday, 18 September 2006, 18:37 GMT
Task Type Patches
Category Drivers
Status Closed
Assigned To No-one
Operating System iPod Nano
Severity Low
Priority Normal
Reported Version
Due in Version Undecided
Due Date Undecided
Percent Complete 100%
Votes 0
Private No

Details

This patch is a quick hack to support the auto pause feature of the original Apple firmware. Rockbox _SHOULD_ pause now automatically. This patch was made for my iPod nano and I guess the G5, too, since it's mostly the same machine. It does not yet work properly and is just a quick hack, but please complain about it and test it. Own risks of course. I made this because of a current feature request.
This task depends upon

Closed by  Linus Nielsen Feltzing (linusnielsen)
Tuesday, 26 September 2006, 10:03 GMT
Reason for closing:  Accepted
Comment by Max Ried (bot47) - Thursday, 23 March 2006, 20:31 GMT
A reworked approach to add the functionality to another file is attached. Still the weired problem that I cannot pause more the one time? Do I do something wrong or is it a general issue?


Apply only pause2.diff. pause.diff is obsoleted
Comment by Max Ried (bot47) - Thursday, 23 March 2006, 20:52 GMT
http://www.rockbox.org/tracker/task/4901

Works properly if playback.c is reverted to an older version, because pause is just broken.
Comment by Max Ried (bot47) - Friday, 24 March 2006, 14:05 GMT
This time, it's a final approach. It is now able to detect the earphone properly by checking bit 7 of GPIO A. Patches apps/playback.c
Don't complain about the dirty patch, it will apply anyway. There was no way to provide a better one because the CVS Server lacks *grr*
Comment by Max Ried (bot47) - Friday, 24 March 2006, 19:29 GMT
And the last and final one to reproduce Apple behaviour.
Comment by Robert Keevil (obo) - Sunday, 26 March 2006, 01:59 GMT
Version to resume when plugged back in, and config to make the whole thing optional.
Comment by Robert Keevil (obo) - Sunday, 26 March 2006, 19:54 GMT
Settings bits now in the right place.
Should apply cleanly post tagcache
Comment by Dave Chapman (linuxstb) - Monday, 27 March 2006, 17:23 GMT
A small comment about the patch. IMO, you shouldn't use CONFIG_CPU==PP5020 to surround your new code - this feature is not dependent on the type of CPU inside the DAP, it's dependent on the design of the whole DAP. So maybe something like #if defined(IPOD_ARCH) && !defined(SIMULATOR) (assuming it works on all ipods). Or even better, define HAVE_HEADPHONE_DETECTION (or similar) in firmware/export/config-*.h and also create a macro/function called (for example) headphones_inserted() instead of hard-coding the GPIO check.
Comment by Max Ried (bot47) - Monday, 27 March 2006, 17:55 GMT
I don't know about your code policy, just to write senseful code, it should not have such a macro or a HAVE_HEADPHONE_DETECTION flag, since this check is only senseful in one place and there is no need for such a macro. And apart from that, neither the nano nor the video got a PP5020, but is specified as such one. The code will only work on nano and video if the iPod Linux GPIO documentation is correct.
Comment by Robert Keevil (obo) - Monday, 27 March 2006, 20:42 GMT
Use a 3 state config (off, pause only, pause and resume). HAVE_HEADPHONE_DETECTION is defined for nano and video - does it work on 3g or 4g? headphones_inserted() function in button.c - ammicon mentioned it might be possible to detect headphone presence on some archos models as well?
Comment by Robert Keevil (obo) - Monday, 27 March 2006, 22:00 GMT
Should patch clean again... also must learn how to spell amiconn
Comment by Max Ried (bot47) - Tuesday, 28 March 2006, 18:07 GMT
it will not work on older ipods?
Comment by Robert Keevil (obo) - Tuesday, 28 March 2006, 19:09 GMT
I don't know - I've only got a 5g here to test on, so I played it safe... Any 4g users out there who can view what the I/O ports in the debug menu do when headphones are removed?
Comment by Rob (biffhero) - Wednesday, 29 March 2006, 02:17 GMT
I have always thought that a 3 second rewind (if pause_on_remove is set) would be cool. That is because I am constantly unplugging my ipod instead of pausing it first when leaving the car or getting in to the car. If I enter the car, I have to unplug it from my headphones, then plug it in to my car. I listen almost exclusively to podcasts, and always back up a few seconds to get my place back.
Comment by glam (b00st4) - Wednesday, 29 March 2006, 18:41 GMT
hm, doenst work with new cvs...complication with the lang file maybe...

why is that not embedded tho cvs yet, such a good patch!
Comment by Robert Keevil (obo) - Wednesday, 29 March 2006, 19:21 GMT
Should patch clean again
Comment by Robert Keevil (obo) - Thursday, 30 March 2006, 18:33 GMT
Patch now includes optional rewind duration of 0 to 15 seconds.
I don't know if the method I use to rewind is the right thing to do, but it has worked for me so far.
I'm open to suggestions for any improvements to the strings in english.lang - at the moment I'm just trying to keep them as short as possible.
Comment by glam (b00st4) - Thursday, 30 March 2006, 19:59 GMT
id like to see light on for a second on plugin, like in apple firmware, that would also be cool on turning hold off...is it possible?
Comment by Robert Keevil (obo) - Thursday, 30 March 2006, 20:29 GMT
Yup, that makes sense. Backlight will now come on when it detects the headphones have been reinserted.
Comment by glam (b00st4) - Thursday, 30 March 2006, 21:08 GMT
perfect, i have no more additions to this, id say ready for cvs :DDD
Comment by Robert Keevil (obo) - Saturday, 01 April 2006, 16:38 GMT
Updated against todays lang changes.
Comment by Robert Keevil (obo) - Tuesday, 04 April 2006, 18:11 GMT
Add option to disable boot time auto-resume if headphones aren't present
Comment by Robert Keevil (obo) - Tuesday, 04 April 2006, 23:00 GMT
Fix problems with the auto-resume changes??
Comment by Mikael Magnusson (mikaelh) - Wednesday, 05 April 2006, 08:32 GMT
okay, i found the problem after i tried 11244 or so again and stared at the code for 10 minutes. In that version, the headphone block is last before the switch (ev.id) bit, but in this last version, it is before a code bit that writes to ev again. Putting it after that makes it work again. Is there any reason you moved it?
Comment by Mikael Magnusson (mikaelh) - Wednesday, 05 April 2006, 08:37 GMT
Hm, i just noticed that startup resume fails spectacularly if you move the block as i described. Do you have a solution?
Comment by Mikael Magnusson (mikaelh) - Wednesday, 05 April 2006, 09:12 GMT
sorry for spamming a bit, but at least now i think i solved the problem :). You can keep the code where it is, but change ev.id=... and ev.data=... into
queue_post(&audio_queue, Q_AUDIO_RESUME, (bool *)true);
and
queue_post(&audio_queue, Q_AUDIO_PAUSE, (bool *)true);
, this way everything seems to work, and it feels like it makes more sense too. In the early versions you were eating real events from the queue and in the latest you were overwritten by them.
Comment by glam (b00st4) - Friday, 07 April 2006, 12:11 GMT
how do you mean? cant find those values in the patch...
Comment by Robert Keevil (obo) - Friday, 07 April 2006, 13:30 GMT
It's the
+ ev.id = Q_AUDIO_PAUSE;
+ ev.data = (void *)1;
lines (plus the Q_AUDIO_RESUME pair further down) in playback.c

But apart from that, the patch is broken at the moment, after the recent playback code changes.
It needs changing anyway (the pause/resume code shoudn't be where it is - but I'm not sure where it should be instead)
It needs to act whereever you are in RB... (so from that point of view playback.c makes sense).
Should it be setup as a BUTTON_ for the platforms that have headphone detection?
Comment by Robert Keevil (obo) - Saturday, 15 April 2006, 19:00 GMT
Patch against latest CVS.
Still not as it should be, but working again.
Comment by Max Ried (bot47) - Saturday, 15 April 2006, 19:32 GMT
Realy, it's time for it to be commited into CVS. Why don't they do so?
Comment by Robert Keevil (obo) - Saturday, 15 April 2006, 19:42 GMT
Because Rockbox is in feature freeze, and the patch isn't doing things the "Right Way"(TM)

Inserting/removing the phones should cause an event, instead of sitting in the playback thread looking to see if it's changed.
Comment by Max Ried (bot47) - Sunday, 16 April 2006, 08:27 GMT
OK, so it's a question of officialism. Too bad. If you know how to implement it the right way, please let me know and I will try it myself.
Comment by Robert Keevil (obo) - Sunday, 16 April 2006, 10:10 GMT
Well....

I've been trying to make the phone socket generate events by changing the interrupt code in firmware/drivers/button.c (line 625-634) - by changing the 0x20 values to 0x80 (ie purely socket) or 0xA0. I've also tried adding new interrupts for GPIOF, value 0x20 (which is generated by the headphone socket on my 5g - I think this maybe different on other models?). I've been using my piezo patch (5111) to enable beeps when interrupts occur - no luck so far.

I'm a bit stuck at this point as to what to try next. I'm not even sure that this is the right way to go - some extra feedback would be nice! :)
Comment by Max Ried (bot47) - Sunday, 16 April 2006, 13:54 GMT
http://ipodlinux.org/GPIO describes the GPIOs for a lot of iPod Generations. Maybe I'm too stupid for C but in general
if (GPIO_INPUT_VALUE_OR_WHAT_EVER & 0xWHAT_EVER)
never works in my tests.
Comment by Robert Keevil (obo) - Tuesday, 23 May 2006, 21:23 GMT
Scrape off the bitrot - no other changes.
Comment by Robert Keevil (obo) - Thursday, 08 June 2006, 17:37 GMT
bitrot again
Comment by Robert Keevil (obo) - Friday, 23 June 2006, 21:39 GMT
Remove patch code from playback.c and add to new file. Run unplug process in its own thread, which sleeps for HZ/2 per cycle.
Solves slow response to headphone induced pause/resume while the playback system is filling the codec buffer.
Comment by Reza (afruff23) - Friday, 14 July 2006, 03:47 GMT
Is it possilbe to port this to non-iPod players or do they not have the ahrdware to do so?
Comment by Dominik Riebeling (bluebrother) - Friday, 14 July 2006, 06:42 GMT
AFAIK (and can see from the patch) this will only work on ipod devices as they use a GPIO pin to detect the earplugs, so other players are missing the hardware.
Comment by Robert Keevil (obo) - Friday, 14 July 2006, 12:40 GMT
Yes, the headphones_inserted() function added to button.c is specific to ipod hardware (the rest should be fine across all platforms), but... Amiconn mentioned some months ago in IRC that it might be possible to detect the presence of headphones for Archos devices. It /might/ also be possible on other targets - an easy way to check is to go into the "View I/O ports" option in the debug menu, and see if there is a consitant change on insert/removal (and not caused by other things like backlight control).
Comment by Robert Keevil (obo) - Tuesday, 18 July 2006, 17:34 GMT
Reverse the logic of the headphones_inserted() function so it actually matches the function name.
Comment by Jesus Climent (mooch) - Monday, 11 September 2006, 20:51 GMT
Is there any way this patch could be applied right now? the pause-on-unplug is a really cool feature..
Comment by Robert Keevil (obo) - Monday, 11 September 2006, 21:03 GMT
Updated the patch.

Thread is now created/removed as needed - no longer any need to reboot after changing the settings.

With any luck this patch might even be looked at one day.
Comment by Max Ried (bot47) - Tuesday, 12 September 2006, 12:58 GMT
Why don't they commit it?
Comment by Ryan Sawhill (ryran) - Tuesday, 12 September 2006, 13:01 GMT
Thanks for the update Robert. I really appreciate all your work on this patch. :)

Obviously, I too hope it's committed some day soon.
Comment by Robert Keevil (obo) - Sunday, 17 September 2006, 15:25 GMT
Updated for the new scheduler.
Comment by Robert Keevil (obo) - Monday, 25 September 2006, 19:58 GMT
Rework of the patch following feedback from Linus and Amiconn.
Comment by Robert Keevil (obo) - Tuesday, 26 September 2006, 09:08 GMT
Remove unnecessary #ifdef in headphones_inserted()

Loading...