Hi Martin,
I've applied your patch to SVN, thanks for the corrections! You get
bonus points for using a const member function as well, something that
doesn't get used nearly enough:
+++src/trackinfoobject.h (working copy)
+ bool getHeaderParsed() const;
(I wasn't aware of this C++ nuance until recently - When the "const"
keyword is used after a member function declaration, it makes sure the
function doesn't modify any member variables that belong to the class.
For example, the const above ensures at compile time that
getHeaderParsed() doesn't change anything inside the TrackInfoObject.
Cool!)
Thanks again,
Albert
On Sun, 2008-05-11 at 16:52 +0200, Martin Sakmar wrote:
> Hi,
> I just made some other changes to this. You can try this patch instead
> of that I sent before.
>
> On Sun, May 4, 2008 at 6:57 PM, Albert Santoni <[EMAIL PROTECTED]>
> wrote:
> Hi Martin,
>
> I have a few comments and questions about your patch.
>
> On Tue, 2008-04-29 at 18:49 +0200, Martin Sakmar wrote:
>
>
> > Hi,
> >
> > Most of my mp3s have BPMs stored in tag, so I made some
> changes to
> > read and use them. Additional changes are especially related
> to
> > minimum and maximum BPM values in BpmDetect class. When
> maxBpm is
> > lower than 2*minBpm (passed to BpmDetect constructor), then
> > BpmDetect::getBpm() will sometimes return 0. So default
> values are
> > used in this constructor (defined in bpmdetect.h) and
> commented out
> > values passed to it from BpmScheme in BpmDetector. BPM is
> corrected
> > with correctBPM to be within the limit specified by
> BpmScheme (or
> > little more, rather than 0).
>
>
> Ok, so the quirk that the initial BPM detection range must be
> greater
> than {min, min*2} is something that was in Mixxx before your
> patch,
> correct?
>
> I added a checkbox to dlgprefbpm to enable or disable this. Maybe
> correctBpm could be modified to return zero if the BPM can't be within
> the range, but I prefer returning a BPM if it can be detected.
>
>
>
> We've received many requests for Mixxx read to BPMs from ID3
> tags, so I
> think many people are going to appreciate this.
>
> In track.cpp, in slotLoadPlayer1(), your patch has this chunk:
>
> @@ -886,6 +888,8 @@
> // Request a new track from the reader:
> m_pBuffer1->getReader()->requestNewTrack(m_pTrackPlayer1,
> bStartFromEndPos);
>
> + SoundSourceProxy::ParseHeader(m_pTrackPlayer1);
> + if(m_pTrackPlayer1->getBpm())
> m_pTrackPlayer1->setBpmConfirm(true);
> // Detect BPM if required
> if (m_pTrackPlayer1->getBpmConfirm()== false ||
> m_pTrackPlayer1->getBpm() == 0.)
> m_pTrackPlayer1->sendToBpmQueue();
>
> Why do you call ParseHeader() here? As best as I can tell,
> ParseHeader()
> gets called when a new TrackInfoObject is created.
>
> ParseHeader is required here to read BPM from tag on load. It's called
> when TrackInfo is created from filename, but not when you load a song
> from library. I added a bool to TrackInfoObject which is set to true,
> when header is parsed, and then it won't be called again in
> slotLoadPlayer if it already was called.
>
>
>
> Other than that, everything looks pretty good. If you can shed
> some
> light on my two questions, I'd be happy to slip this into SVN
> before the
> beta3 release.
>
> Thanks,
> Albert
>
>
> Regards,
> Martin
>
>
-------------------------------------------------------------------------
This SF.net email is sponsored by the 2008 JavaOne(SM) Conference
Don't miss this year's exciting event. There's still time to save $100.
Use priority code J8TL2D2.
http://ad.doubleclick.net/clk;198757673;13503038;p?http://java.sun.com/javaone
_______________________________________________
Mixxx-devel mailing list
[email protected]
https://lists.sourceforge.net/lists/listinfo/mixxx-devel
