Closed GoogleCodeExporter closed 8 years ago
I discussed this issue with Michael Coffey in private email before I posted this
issue and he asked me to include the texts of our conversations in this thread.
Here
are the messages:
Michael Coffey:
Are you sure that you need to put it in a separate thread? As far as I knew,
thread
priority is only relative within a process and it's the process priority that
really
matters. If another process is higher priority and is hogging the CPU, there
is not
much you can do about it. The active scheduler doesn't have much to do with
this.
If the audio was skipping due to things we are doing in our thread then I could
see
that it could be an active scheduler usage problem, but our problem is because
of
another process.
Have you tried giving the main thread a priority of real time without changing
anything else? Maybe this thread priority has some special properties.
Having said this, if it works, it works. I'm just trying to make sure we don't
make
things too complicated unnecessarily.
Steve Punter (pbextreme):
>Are you sure that you need to put it in a separate thread? As far as I knew,
>thread priority is only relative within a process and it's the process priority
>that really matters.
I couldn't find anything in the documentation to suggest that the priority of a
THREAD was affected by the priority of its spawning process. This isn't the
case any
other operating system I've programmed for, nor does it appear to be case with
Symbian either.
>Have you tried giving the main thread a priority of real time without changing
>anything else? Maybe this thread priority has some special properties.
The API call to change the priority of a PROCESS (I.E you running application)
is
restricted to no higher than EPriorityHigh. The documentation says that if you
try to
apply a value higher than this, it will panic. I proved that by attempting to
set the
priority of the application to EPriorityRealtimeServer, but like they said, a
panic
occurred and the application closed. There is NO restriction on the priority of
a
thread, and you can set that priority INDEPENDENTLY of your application's
priority.
>If another process is higher priority and is hogging the CPU, there is not much
>you can do about it. The active scheduler doesn't have much to do with this.
That's true, but the explanation of the problem was that because the stream
engine
used Active Objects to schedule its access to the hardware it was at the mercy
of
other Active Objects running in your SYSTEM. Here's an excerpt from a message I
found
on this subject:
>> If you want latency free streaming audio with no glitches due to other
active objects
>> running on your system, then you absolutely MUST implement the audio
streaming in
>> a high priority (preferably real-time priority) worker thread ... trust me,
it
does work,
>> I'm currently doing real-time multi-threaded audio on multiple platforms.
>> It is totally necessary.
>Having said this, if it works, it works. I'm just trying to make sure we
don't make
>things too complicated unnecessarily.
It DOES work, like a charm. As for the complexity, that's mostly locked up in
the
classes I've written. The modifications to your existing code are relatively
minor by
comparison. That's why I created the CAudioControl class. It does all of the
necessary work to interface with the thread (such as using Mutexes to serialize
access to shared data).
Michael Coffey:
Each thread has its own active scheduler, so other active objects in the
"system"
don't matter. It's just whether your thread gets to run or not.
I guess that the benefit of putting this in another thread is that if another
process
hogs the CPU, when our process get its time slice, that time will be spent on a
thread that is dealing only with audio playback rather than a thread that is
dealing
with downloading mp3 data or updating the screen, etc as well.
Steve Punter (pbextreme)
Original comment by pbextreme@gmail.com
on 20 Dec 2008 at 6:18
Thanks for this.
When you're happy with your changes you should submit them to your branch so
that
they can be tested and reviewed by the rest off us.
Cheers!
Original comment by eartle@gmail.com
on 20 Dec 2008 at 6:23
Oh, and I thought applets were a java thing?
Original comment by eartle@gmail.com
on 20 Dec 2008 at 6:24
I presently have the changes installed in my HIGHLY-MODIFIED version 0.3.1.
What I
play to do, once I'm happy with the code, it to install it into a virgin copy
of the
0.3.2 source code and then submit that. There are quite a few changes necessary
to
the mobblerradioplayer class and I don't want these confused with anything else
I've
added to the code.
Original comment by pbextreme@gmail.com
on 20 Dec 2008 at 6:25
Applet is probably a Java term. What do we call native Symbian apps then?
Original comment by pbextreme@gmail.com
on 20 Dec 2008 at 6:26
Sounds good to me. I look forward to seeing what you've done.
I believe you just call them apps.
Original comment by eartle@gmail.com
on 20 Dec 2008 at 6:47
I've now added my modifications for this issue to my BRANCH. This version
contains
ONLY changes to support the thread-based sound engine, thus it should be pretty
easy
to find out what I did to add the support.
There are a handful of new classes and most of the modification to the original
source code takes place in the MobblerRadioPlayer class.
Compile this version and give it a try.
Original comment by pbextreme@gmail.com
on 22 Dec 2008 at 3:42
I had to make a few changes to get it to build, but I've tested it and it's
working well.
The worst skipping I've experienced is when I scroll up and down in the
browser, but
I can do that now without a problem.
I've made some line-by-line and general code review comments on your revision.
It
would be great to get these fixed up and make another release as soon as we are
sure
that the change is stable.
Original comment by eartle@gmail.com
on 22 Dec 2008 at 9:14
What's the standard procedure to proceed from here. Is it up to me to make the
changes based on the comments, or will you make the changes to bring the code
into
compliance with your requirements. I'm fine with it either way, but I'd like to
know
what is expected from me so that we can move forward.
Original comment by pbextreme@gmail.com
on 22 Dec 2008 at 9:57
Another "interesting" thing I can do on my N95 8GB with the new version is that
I can
play music in the background while I listen to something else running on a
different
program. For example, I listened to a Podcast on the built-in MP3 player while
Mobbler simultaneously played music in the background. I was also able to watch
(and
listen to) YouTube videos via Mobitubia. Perhaps the ability to allow multiple
applications to use the sound hardware simultaneously is restricted to the
N-series,
but I don't have any other Nokia phones to try it on.
Original comment by pbextreme@gmail.com
on 22 Dec 2008 at 10:00
As a project member it is expected that you would make the code review updates
until
the code is ready to be merged back to the trunk. You should also be
responsible for
merging changes committed by other people from the trunk to your personal
branch.
Of course, you are not obligated to do any of this, but please let me know
either way.
Someone raised an issue a while ago (Issue 123) that both Mobbler and the S60
music
player can play at once. I don't see why anyone would want to do this, but we
should
check that it doesn't mess the scrobbling up. I think that Mobbler should
probably
stop the radio if the music app starts playing music and not allow you to start
a
radio station while the music player is currently playing.
Original comment by eartle@gmail.com
on 22 Dec 2008 at 10:18
I'm happy to follow your direction on this. Like I said, it's your project and
I feel
obligated to follow any instructions you give me. I just wasn't sure how I was
SUPPOSED to proceed. I will of course make all requested modifications to my
code,
though I can't guarantee that will happen until after Christmas, since things
are
getting really busy around here as a result of the holidays. You never know, I
might
have the time to do it before then.
I hadn't really given any thought to the Scrobbling issue when both Mobbler and
the
music player are running simultaneously, since like you I can't image people
trying
to listen to 2 pieces of music at once. However, listening to music in the
background
while listening to a Podcast seems quite natural.
Original comment by pbextreme@gmail.com
on 22 Dec 2008 at 11:28
Oh yeah, I forgot to ask. What changes did you have to make to get my code to
build
(or is that mentioned in the comments you added to the code)?
Original comment by pbextreme@gmail.com
on 22 Dec 2008 at 11:30
Oh. You hadn't submitted your mmp file changes.
There were two other changes, but these would be fixed by merging the trunk to
your
branch. They were that resource file problem you had and there were some
incompatibilities with building using the 5th edition SDK.
Original comment by eartle@gmail.com
on 22 Dec 2008 at 11:40
Actually, there were two additional LIBs included in my code to support the
equalizer. The CAudioControl and the CAudioThread classes have the equalizer
capability built-in, but I didn't implement them in the code I sent you.
Original comment by pbextreme@gmail.com
on 23 Dec 2008 at 12:03
Because I'm new to this, I don't understand where the "code review updates" you
mentioned are. I went to Tortoise SVN and I asked it to update my files, but it
didn't change anything. Perhaps I've misunderstand how this works. Thanks in
advance.
Original comment by pbextreme@gmail.com
on 23 Dec 2008 at 12:12
Some Google Code tips:
You can see the list of revisions by clicking the Source tab and then Changes:
http://code.google.com/p/mobbler/source/list
You can automatically links to issues just by writing it like this: issue 196,
and
to revisions by writing it like this: r188.
You can see the line-by-line code review comments by clicking diff next to each
file, or by expanding the "[+] X line-by-line comments" bit.
We can also use that diff method to see the changes you've made.
If you've made changes to your code locally, and want to update them to your
branch
on Google Code, right click and select SVN Commit... It gives you a chance to
review
your changes and add a comment. (SVN Update works the other way: it checks if
there's newer code in the repository than you have locally, and will update
your
newer copy.)
I've changed the status of this issue from New to Started, to indicate you've
made a
start on this :)
Original comment by hugovk@gmail.com
on 23 Dec 2008 at 8:30
I've just checked-in the newest copy of my code. I THINK I managed to change
everything I was asked to do in the first release.
Original comment by pbextreme@gmail.com
on 23 Dec 2008 at 8:41
Sorry, but the last check-in was spread across 2 updates (194 and 195) because I
forgot to add some files in 194.
Original comment by pbextreme@gmail.com
on 23 Dec 2008 at 8:48
I think that this is all looking good. I think we should get this change into
the
trunk as soon as possible. We can do minor updates to the details later, if we
want
to, but I am happy that it is all working.
How familiar with SVN are you, pbextreme? Normal practice is to merge any
changes
from the trunk to your branch so that the only differences between the two
branches
are your changes. Make sure it all still works and then merge back the other
way to
get your changes into the trunk. There should then be no difference between
the two
branches. Let me know if you have any problems with this. It might be better
to
discus this kind of thing over gmail chat.
Original comment by eartle@gmail.com
on 23 Dec 2008 at 9:26
I installed Tortoise SVN as part of a contract I did over a year ago, but I've
had
little experience with it otherwise. I would certainly appreciate any help you
can
give me so that I'll be using it in the correct manner.
I'm glad you're happy with the changes. I tried to conform to all of the styling
issues you raised in your comments. While I still believe the style is ugly,
I'm also
familiar with group projects and I understand the need for ALL members of the
group
to comply with the styling template laid out by the project manager. If it were
my
project, things would be different, but it's not and I will endeavor to make all
future contributions match the styling guidelines BEFORE I send them to you in
the
first place.
Original comment by pbextreme@gmail.com
on 23 Dec 2008 at 9:34
P.S. pbextreme is my username only because my email address used to open the
account
here is pbextreme@gmail.com. My name is actually Steve Punter. I'd appreciate
it if
you could at least call me Steve instead of pbextreme. Thanks.
Original comment by pbextreme@gmail.com
on 23 Dec 2008 at 9:42
Again, I think that the important thing is that there is a style, not which
stlye.
As we are programming in Symbian C++ it is easiest to fall back on their style
instead of creating our own. It should also be consistent with most Symbian
examples
out there.
I've only been using SVN since I first committed the code here, but am a regular
perforce user. I read up on a thing or two about SVN a while ago so I know a
little
bit about how it should work in theory, but have a few problems in practice.
I have been calling you pbextreme because that is your username and is how you
are
identified everywhere on google code. If I referred to you Steve, people not
part of
the project would wonder whom I am talking to or about. I think that it is
best to
use usernames on this site to avoid confusion.
Original comment by eartle@gmail.com
on 23 Dec 2008 at 10:08
If you feel that calling me pbextreme is better for everyone involved, then so
be it.
However, if I'd known in advance that I'd have to go by this name I would have
created a better gmail address before I was added to the project.
Also, I received a private email from you to join Google Chat, but when I log
into it
I'm told there was a problem with the link I followed to get there. I've tried
pasting it by hand, but the same thing happens.
Original comment by pbextreme@gmail.com
on 23 Dec 2008 at 10:12
You can always create a new gmail account and swap over, if you'd prefer. I
think I
sent the gmail chat invite to the wrong email address, I've sent it to the
gmail one now.
Original comment by eartle@gmail.com
on 23 Dec 2008 at 10:15
Michael has changed my username on here and so now pbextreme is:
steve.j.punter
That's my real name, including my middle initial. Hopefully this won't create
too
much confusion.
Original comment by steve.j.punter@gmail.com
on 23 Dec 2008 at 10:57
Whom is Michael? ;)
Original comment by eartle@gmail.com
on 23 Dec 2008 at 11:17
Sorry, I meant eartle :-)
Original comment by steve.j.punter@gmail.com
on 23 Dec 2008 at 11:48
Finally given this a run on an N95 8GB and it's playing very smoothly indeed,
even
when scrolling pages with Opera Mini that used to cause stutters before. Great
work
steve.j.punter!
Original comment by hugovk@gmail.com
on 24 Dec 2008 at 9:30
Good stuff! Those multi-tasking guys are going to love this.
I think I will have a play with making some edits to this myself after
Christmas.
Mainly for my own nerd purposes. (I noticed some 'busy waiting' when stopping
the
track, and I'd like to have a play with the shared memory stuff, etc, etc.)
Original comment by eartle@gmail.com
on 24 Dec 2008 at 11:44
Good work guys!
Any binaries available for us users to test? :)
Original comment by 1giglimit@gmail.com
on 26 Dec 2008 at 9:42
1giglimit -- no binaries available yet, there are still a few wrinkles to be
ironed
out (issue 201, issue 202).
Original comment by hugovk@gmail.com
on 27 Dec 2008 at 10:17
Just to let you know that I've started work on changing this a little bit.
I'm going to make the main thread wait for the audio thread to respond to each
command by adding a rendesvous between the two threads. This is because it is
currently possible for commands to get lost if the audio thread hasn't had a
chance
to deal with a command and then a second one is sent.
I'm going to get rid of the active object that handles the commands in the audio
thread because CMobblerAudioThread can be an active object and deal with that
itself.
I'm going to add a way for the audio thread to signal to the main thread that
draw
deferred should be called so that we can get rid of that active object that
calls
itself back endlessly.
I'm going to make it so that the audio thread only ever plays one track. Next
track
is then handled by the radio player by creating a new audio thread. It is more
simple for that next track behaviour to be hanled there and means that we can
start
buffering a new track whilst the current one is still playing.
And some other minor simplifications.
Original comment by eartle@gmail.com
on 29 Dec 2008 at 11:08
We'll see if your changes can solve some of the problems HUGOKV has been having
that
I just can't repoduce at my end.
Original comment by steve.j.punter@gmail.com
on 29 Dec 2008 at 3:47
Here's something very weird. After weeks of NEVER seeing a USER 44 or USER 45,
I
started getting them left, right, and center. I couldn't figure out what I'd
done,
and so in desperation I rebooted the N95. Once I did that the problem WENT
AWAY. Go
figure.
Original comment by steve.j.punter@gmail.com
on 31 Dec 2008 at 7:16
I think I've solved the memory allocation issue. I've changed it so that all
the
audio data is allocated and freed in the audio thread. We are not sharing any
RArrays either.
I've also changed it so that each track is played in a different thread and we
start
buffering the next track before the previous one has finished so there should
be no
buffering gap in between songs (except for at the boundary of a playlist, but I
might
have a look into changing that too).
I think I've messed up the equalizer stuff though as I wasn't paying much
attention
to that.
Original comment by eartle@gmail.com
on 31 Dec 2008 at 11:54
I'm looking forward to seeing how you re-organized the code, but at the time I
read
your message it wasn't yet available in your branch, nor in the trunk. Let us
know
when you put these changes up somewhere for us to try.
Original comment by steve.j.punter@gmail.com
on 31 Dec 2008 at 3:11
I'm planning on doing a little bit of finishing off and testing tomorrow and
will
then submit to the trunk.
Original comment by eartle@gmail.com
on 31 Dec 2008 at 3:34
I've just submitted my update to this in r218. There are a lot of changes as I
got a
bit carried away, but it's essentially the same except for the downloading of
the
next track a little bit early.
I think I've removed all the memory management issues we were having, but it
would be
great to get some testing and code reviewing of this as it seems to be quite a
trick
thing to get working properly on different devices etc.
Original comment by eartle@gmail.com
on 1 Jan 2009 at 7:07
I compiled and installed the newest trunk version of Mobbler on my N95 8GB and
it
seemed to work fine until I pressed the right arrow key to skip to the next
track. I
then got a KERN-EXEC 0 panic, though it didn't shut down the application. The
next
track did play (after a short delay). When I tried to exit the program I got
another
KERN-EXEC 0, but the application DID EXIT.
Original comment by steve.j.punter@gmail.com
on 1 Jan 2009 at 8:39
Damn.
I'm not getting that, but then I haven't got extended panic codes enabled on my
phone. I never see panics on my phone. Do you have this enabled?
If it is panicking, but carrying on then it must be a panic in the audio
thread. The
panic code and circumstances suggest that a resource is being used that has been
closed or not opened whilst it the thread is shutting down. Shouldn't be too
hard to
debug once I can reproduce it.
Original comment by eartle@gmail.com
on 1 Jan 2009 at 9:53
Yes I do have extended panics turned on in my phone. Since it occurs without
stopping the application, it wouldn't be visible on a phone without extended
panics
enabled. I agree it sounds like an issue shutting down the old audio thread
once
we're finished with it.
Original comment by steve.j.punter@gmail.com
on 1 Jan 2009 at 9:56
This should be fixed now in r219. I reproduced it with exteneded panic codes
enabled
on my phone. I found that it was because the equaliser was being deleted after
the
audio output stream. The equliser must try to use the output stream in its
destructor. I also a problem with stopping the active scheduler twice which
has been
solved.
I've made Mobbler fetch the next radio playlist when the last song in the
current
playlist starts. There should now never be a gap between songs if your data
connection is good and you listen to the all the way through.
Original comment by eartle@gmail.com
on 2 Jan 2009 at 4:06
It seems to work well here. I'll therefore begin implementing the Graphics
Equalizer
code and I'll let you know when I've got it ready.
Original comment by steve.j.punter@gmail.com
on 2 Jan 2009 at 5:49
Excellent!
I've been using it non-stop for an hour or two now and am really enjoying the
next
track starting immediately and using other apps without playback skipping at
all.
Good stuff!
However, on one occasion it switched to a new track and was downloading it, but
didn't start playing it even though more than half the track downloaded. There
will
need to be a minor fix in the next track ligic if I can work out the
circumstances
that made this happen, but it hasn't happened again in a long time now.
Original comment by eartle@gmail.com
on 2 Jan 2009 at 6:06
I've made another update which I think fixes the issue where the next track
would
start, but not play.
I also started getting KErrAccessDened every time the next audio thread tried
to open
its audio stream. I think it is because there is a multimedia server policy
about
having many audio streams open at the same time. We now wait for the audio
thread to
be set as the current one before opening the audio stream.
I'm thinking that should be everything now. Perhaps we'll leave it for a few
more
days of testing to make sure it's OK and then make a new release.
Original comment by eartle@gmail.com
on 3 Jan 2009 at 10:52
Check to see if the access denied problem doesn't hit you when opening the
Equalizer
sub-menu. It also opened a stream, though only temporarily in order to open an
equalizer object to get the names of the profiles.
Original comment by steve.j.punter@gmail.com
on 3 Jan 2009 at 2:52
Bad news, but I just got a USER 44 error while testing out the newest code.
This
stopped the current song from playing, but the next one started okay, thus
demonstrating that the error occurred in the audio thread.
I uninstalled that version and I reinstalled my highly-modified 0.3.1 that uses
my
original thread model. I didn't run into any problems using that WITHOUT
rebooting
the phone.
This error MIGHT have been caused by the removal of the MUTEX, but I can't
prove
that's the case. I'll try putting the MUTEX back in and see what happens.
Original comment by steve.j.punter@gmail.com
on 4 Jan 2009 at 7:04
I ran into that problem you noted earlier, in which the next track did not
play, but
continued to buffer. I don't know if this will help, but when this happened I
noted
that that the previous song ended 5 SECONDS TOO SOON (the size of the
prebuffer). It
actually happened to me multiple times and the same early termination occurred
each
time. Hopefully I didn't create this problem by putting back the MUTEX, but I
can't
see how that would have caused this issue.
Original comment by steve.j.punter@gmail.com
on 4 Jan 2009 at 7:38
Are you sure you have updated to, and are building, the latest version of the
trunk?
It sounds like you are getting problems that have already been fixed.
I don't understand why you think that adding a mutex will solve the USER 44 you
are
getting. The panic means that we are trying to free memory that has not been
allocated or already freed. There is nothing in shared memory that we are
freeing in
the audio thread so I can't see how this would help.
Original comment by eartle@gmail.com
on 4 Jan 2009 at 1:58
Original issue reported on code.google.com by
pbextreme@gmail.com
on 20 Dec 2008 at 6:13