[PATCH] Several fixes/improvements for CD playback

Hello,

Please consider the attached patch, cdda_improvements.diff. It makes
several changes to the CDDA plugin and associated files, explained
below:

1) Apply CD-related gapless patches from
http://hg.debian.org/hg/xine-lib/xine-lib-gapless, avoiding noticeably
flawed track transitions.

Explanation: Despite 'gapless playback', the very start of tracks are
often chopped off on track transitions. I stumbled upon the fix to the
demuxer at the above address (specifically,
http://hg.debian.org/hg/xine-lib/xine-lib-gapless/rev/86b9162cfcfe).
The same basic fix that works for wavs also works for CD audio.

This also permits us to reduce the CDDA frame cache from 500 to 100; it
was originally raised from 100 in an only-partially successfull attempt
to work around the gapless problem. The cache can be brought back down
(improving seek responsiveness) now that a far more correct fix has
been found.

2) Make use of the DGENRE information provided by CDDB. Many frontends
support it; we just weren't bothering to use it. Also improves track
number reporting. Based on an unused patch by Markus Kaufhold in the
bug database.*

3) Complete the fix started in
http://hg.debian.org/hg/xine-lib/xine-lib?cmd=changeset;node=827afe1c3777;style=gitweb

Explanation: Replacing strrchr with strchr to allow disc and track
titles to contain "=" when parsing CDDB is great; this makes a few more
replacements that were missed by the original patch.

Please consider applying these fixes.

Thanks.

* http://sourceforge.net/mailarchive/forum.php?thread_name= \
E1FI2Xj-0001tC-Rk%40sc8-sf-web3.sourceforge.net&forum_name=xine-bugs

[attachment removed]
------------------------------------------------------------------------------
This SF.net email is sponsored by:
High Quality Requirements in a Collaborative Environment.
Download a free trial of Rational Requirements Composer Now!
http://p.sf.net/sfu/www-ibm-com
_______________________________________________
xine-devel mailing list
xine-devel@...
https://lists.sourceforge.net/lists/listinfo/xine-devel

[Darren Salt]

I demand that Christopher Martin may or may not have written...

> Please consider the attached patch, cdda_improvements.diff. It makes
> several changes to the CDDA plugin and associated files, explained below:

> 1) Apply CD-related gapless patches from
> http://hg.debian.org/hg/xine-lib/xine-lib-gapless, avoiding noticeably
> flawed track transitions.

NAK. Your patch should be against that repository; however, I'll merge that
into the main repository.

[snip]
> 2) Make use of the DGENRE information provided by CDDB. Many frontends
> support it; we just weren't bothering to use it. Also improves track number
> reporting. Based on an unused patch by Markus Kaufhold in the bug
> database.*

Would be better to build on top of that patch, then; should be a separate
patch.

> 3) Complete the fix started in
> http://hg.debian.org/hg/xine-lib/xine-lib?cmd=changeset;node=827afe1c3777;style=gitweb

> Explanation: Replacing strrchr with strchr to allow disc and track titles
> to contain "=" when parsing CDDB is great; this makes a few more
> replacements that were missed by the original patch.

Should be a separate patch.

Providing this as a series of patches on top of xine-lib-gapless would be
preferred. (You should include Markus Kaufhold's patch in the series.)

I suggest that you commit them locally then use "hg email".

--
| Darren Salt    | linux or ds at              | nr. Ashington, | Toon
| RISC OS, Linux | youmustbejoking,demon,co,uk | Northumberland | Army
| + Use more efficient products. Use less.          BE MORE ENERGY EFFICIENT.

IC: Understanding as in 'Oh, IC'.

------------------------------------------------------------------------------
Crystal Reports - New Free Runtime and 30 Day Trial
Check out the new simplified licensign option that enables unlimited
royalty-free distribution of the report engine for externally facing
server and web deployment.
http://p.sf.net/sfu/businessobjects
_______________________________________________
xine-devel mailing list
xine-devel@...
https://lists.sourceforge.net/lists/listinfo/xine-devel

On April 25, 2009 14:03:32 Darren Salt wrote:

> I demand that Christopher Martin may or may not have written...
>
> > Please consider the attached patch, cdda_improvements.diff. It
> > makes several changes to the CDDA plugin and associated files,
> > explained below:
> >
> > 1) Apply CD-related gapless patches from
> > http://hg.debian.org/hg/xine-lib/xine-lib-gapless, avoiding
> > noticeably flawed track transitions.
>
> NAK. Your patch should be against that repository; however, I'll
> merge that into the main repository.

OK, now that you've merged the gapless branch, we still need to:

1) Apply the same fix (just applied to demux_wav.c) to demux_cdda.c
(cdda_improvements1.diff); and
2) Adjust the CDDA frame cache to a 'snappier' value
(cdda_improvements2.diff).

> > 2) Make use of the DGENRE information provided by CDDB. Many
> > frontends support it; we just weren't bothering to use it. Also
> > improves track number reporting. Based on an unused patch by Markus
> > Kaufhold in the bug database.*
>
> Would be better to build on top of that patch, then; should be a
> separate patch.

cdda_improvements3.diff - this is basically the same as Markus
Kaufhold's patch. The bug (and patch) don't seem to exist in the
current bugzilla anymore (but I'd saved it locally a while ago).
However, the original report can be found at:

http://sourceforge.net/mailarchive/forum.php?thread_name= \
E1FI2Xj-0001tC-Rk%40sc8-sf-web3.sourceforge.net&forum_name=xine-bugs

> > 3) Complete the fix started in
> > http://hg.debian.org/hg/xine-lib/xine-lib?cmd=changeset;node=827afe
> >1c3777;style=gitweb
> >
> > Explanation: Replacing strrchr with strchr to allow disc and track
> > titles to contain "=" when parsing CDDB is great; this makes a few
> > more replacements that were missed by the original patch.
>
> Should be a separate patch.

cdda_improvements4.diff (applies on top of cdda_improvements3.diff).

Hope that helps.

[attachment removed]
[attachment removed]
[attachment removed]
[attachment removed]
------------------------------------------------------------------------------
Crystal Reports - New Free Runtime and 30 Day Trial
Check out the new simplified licensign option that enables unlimited
royalty-free distribution of the report engine for externally facing
server and web deployment.
http://p.sf.net/sfu/businessobjects
_______________________________________________
xine-devel mailing list
xine-devel@...
https://lists.sourceforge.net/lists/listinfo/xine-devel

[Darren Salt]

I demand that Christopher Martin may or may not have written...

[snip]
> OK, now that you've merged the gapless branch, we still need to:

> 1) Apply the same fix (just applied to demux_wav.c) to demux_cdda.c
> (cdda_improvements1.diff); and
> 2) Adjust the CDDA frame cache to a 'snappier' value
> (cdda_improvements2.diff).

Committed. I'm leaving the remaining two for the moment; I'm currently
eliminating some code duplication and fixing a bug or two along the way.

[snip]
--
| Darren Salt      | linux at youmustbejoking | nr. Ashington, | Toon
| Debian GNU/Linux | or ds    ,demon,co,uk    | Northumberland | Army
| + Use more efficient products. Use less.          BE MORE ENERGY EFFICIENT.

You will outlive those who seek to destroy you.

------------------------------------------------------------------------------
The NEW KODAK i700 Series Scanners deliver under ANY circumstances! Your
production scanning environment may not be a perfect world - but thanks to
Kodak, there's a perfect scanner to get the job done! With the NEW KODAK i700
Series Scanner you'll get full speed at 300 dpi even with all image
processing features enabled. http://p.sf.net/sfu/kodak-com
_______________________________________________
xine-devel mailing list
xine-devel@...
https://lists.sourceforge.net/lists/listinfo/xine-devel