Rockbox

  • Status Closed
  • Percent Complete
    100%
  • Task Type Patches
  • Category Drivers
  • Assigned To No-one
  • Operating System iPod Nano
  • Severity Low
  • Priority Very Low
  • Reported Version
  • Due in Version Undecided
  • Due Date Undecided
  • Votes
  • Private
Attached to Project: Rockbox
Opened by Max Ried - 2006-03-23
Last edited by Rani Hod - 2006-09-18

FS#4899 - iPod AutoPause

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.

Closed by  Linus Nielsen Feltzing
2006-09-26 10:03
Reason for closing:  Accepted
Max Ried commented on 2006-03-23 20:31

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

Max Ried commented on 2006-03-23 20:52

http://www.rockbox.org/tracker/task/4901

Works properly if playback.c is reverted to an older version, because pause is just broken.

Max Ried commented on 2006-03-24 14:05

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*

Max Ried commented on 2006-03-24 19:29

And the last and final one to reproduce Apple behaviour.

Robert Keevil commented on 2006-03-26 01:59

Version to resume when plugged back in, and config to make the whole thing optional.

Robert Keevil commented on 2006-03-26 19:54

Settings bits now in the right place.
Should apply cleanly post tagcache

Dave Chapman commented on 2006-03-27 17:23

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.

Max Ried commented on 2006-03-27 17:55

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.

Robert Keevil commented on 2006-03-27 20:42

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?

Robert Keevil commented on 2006-03-27 22:00

Should patch clean again… also must learn how to spell amiconn

Max Ried commented on 2006-03-28 18:07

it will not work on older ipods?

Robert Keevil commented on 2006-03-28 19:09

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?

Rob commented on 2006-03-29 02:17

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.

glam commented on 2006-03-29 18:41

hm, doenst work with new cvs…complication with the lang file maybe…

why is that not embedded tho cvs yet, such a good patch!

Robert Keevil commented on 2006-03-29 19:21

Should patch clean again

Robert Keevil commented on 2006-03-30 18:33

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.

glam commented on 2006-03-30 19:59

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?

Robert Keevil commented on 2006-03-30 20:29

Yup, that makes sense. Backlight will now come on when it detects the headphones have been reinserted.

glam commented on 2006-03-30 21:08

perfect, i have no more additions to this, id say ready for cvs :DDD

Robert Keevil commented on 2006-04-01 16:38

Updated against todays lang changes.

Robert Keevil commented on 2006-04-04 18:11

Add option to disable boot time auto-resume if headphones aren’t present

Robert Keevil commented on 2006-04-04 23:00

Fix problems with the auto-resume changes??

Mikael Magnusson commented on 2006-04-05 08:32

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?

Mikael Magnusson commented on 2006-04-05 08:37

Hm, i just noticed that startup resume fails spectacularly if you move the block as i described. Do you have a solution?

Mikael Magnusson commented on 2006-04-05 09:12

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.

glam commented on 2006-04-07 12:11

how do you mean? cant find those values in the patch…

Robert Keevil commented on 2006-04-07 13:30

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?

Robert Keevil commented on 2006-04-15 19:00

Patch against latest CVS.
Still not as it should be, but working again.

Max Ried commented on 2006-04-15 19:32

Realy, it’s time for it to be commited into CVS. Why don’t they do so?

Robert Keevil commented on 2006-04-15 19:42

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.

Max Ried commented on 2006-04-16 08:27

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.

Robert Keevil commented on 2006-04-16 10:10

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 0×20 values to 0×80 (ie purely socket) or 0xA0. I’ve also tried adding new interrupts for GPIOF, value 0×20 (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! :)

Max Ried commented on 2006-04-16 13:54

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.

Robert Keevil commented on 2006-05-23 21:23

Scrape off the bitrot - no other changes.

Robert Keevil commented on 2006-06-08 17:37

bitrot again

Robert Keevil commented on 2006-06-23 21:39

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.

Reza commented on 2006-07-14 03:47

Is it possilbe to port this to non-iPod players or do they not have the ahrdware to do so?

Dominik Riebeling commented on 2006-07-14 06:42

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.

Robert Keevil commented on 2006-07-14 12:40

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).

Robert Keevil commented on 2006-07-18 17:34

Reverse the logic of the headphones_inserted() function so it actually matches the function name.

Jesus Climent commented on 2006-09-11 20:51

Is there any way this patch could be applied right now? the pause-on-unplug is a really cool feature..

Robert Keevil commented on 2006-09-11 21:03

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.

Max Ried commented on 2006-09-12 12:58

Why don’t they commit it?

Ryan Sawhill commented on 2006-09-12 13:01

Thanks for the update Robert. I really appreciate all your work on this patch. :)

Obviously, I too hope it’s committed some day soon.

Robert Keevil commented on 2006-09-17 15:25

Updated for the new scheduler.

Robert Keevil commented on 2006-09-25 19:58

Rework of the patch following feedback from Linus and Amiconn.

Robert Keevil commented on 2006-09-26 09:08

Remove unnecessary #ifdef in headphones_inserted()

Loading...

Available keyboard shortcuts

Tasklist

Task Details

Task Editing