After an evaluation, GNOME has moved from Bugzilla to GitLab. Learn more about GitLab.
No new issues can be reported in GNOME Bugzilla anymore.
To report an issue in a GNOME project, go to GNOME GitLab.
Do not go to GNOME Gitlab for: Bluefish, Doxygen, GnuCash, GStreamer, java-gnome, LDTP, NetworkManager, Tomboy.
Bug 377280 - cdiocddasrc: issue if drive endianness != machine endianness
cdiocddasrc: issue if drive endianness != machine endianness
Status: RESOLVED FIXED
Product: GStreamer
Classification: Platform
Component: gst-plugins-ugly
0.10.4
Other All
: Normal normal
: 1.1.1
Assigned To: Wim Taymans
GStreamer Maintainers
Depends on:
Blocks:
 
 
Reported: 2006-11-20 03:55 UTC by Chris Wang
Modified: 2012-11-27 18:21 UTC
See Also:
GNOME target: ---
GNOME version: 2.15/2.16


Attachments
a possible patch (855 bytes, patch)
2006-12-01 09:32 UTC, Chris Wang
none Details | Review
modified patch (816 bytes, patch)
2007-01-16 02:38 UTC, Chris Wang
reviewed Details | Review
Log of libcdio working properly on GNU/Linux versus Solaris. Suggests how to look for a libcdio bug (2.55 KB, text/plain)
2007-04-24 00:34 UTC, rocky
  Details

Description Chris Wang 2006-11-20 03:55:00 UTC
Please describe the problem:
When play CD with Sound-juicer with a Sparc box, the output is totally garbled noise. 

Steps to reproduce:
1. Place a CD into CDROM and launch Sound-juicer
2. Press play button



Actual results:
The garbled noise outputed to the speaker

Expected results:
The music should be played

Does this happen every time?
yes

Other information:
I figured out that the CDROM with Sparc box is little-endian, while sparc is big-endian, and gstreamer doesn't consider about this endianness issue. I suggest the gstcdiocddasrc.c in gst-plugin-good/ext/cdio directory should take care of this.
Comment 1 Tim-Philipp Müller 2006-11-20 09:44:12 UTC
I can't find API in libcdio to query this, at least not proper libcdio API (only something in the ursurped libcdio-paranoia module). Unless I'm missing something, this sounds like something that needs fixing in libcdio first.
Comment 2 Chris Wang 2006-11-20 09:52:48 UTC
Yes, there has no API in libcdio to do this, unless you use cdparanonia (data_bigendianp). However, I think libcdio should only handle the endianness of CDROM, and should leave gstreamer to handle the endianness of different processor. 

Comment 3 Tim-Philipp Müller 2006-11-20 10:15:12 UTC
> However, I think libcdio should only handle the endianness
> of CDROM, and should leave gstreamer to handle the endianness of different
> processor. 

That's how it works at the moment. GStreamer currently assumes that the data it gets from libcdio is in native endianness and the output caps will reflect this.

So either that assumption is generally wrong (couldn't find anything to that effect in the docs and haven't seen any complaints from Linux/PPC users yet either), or the endianness of the data varies depending on the drive. In the first case, we need to fix things in GStreamer's cdiocddasrc element, in the second case we need libcdio API to provide us with the information about drive/data endianness so we can swap the endianness as appropriate or set the correct output caps.

Comment 4 rocky 2006-11-20 11:09:35 UTC
Yes, libcdio drive endianess reports how the drive gives data back and is not concerned with the processor that is running the code. (Most drives are little endian and supposedly all ATAPI/SCSI drives are little endian also.) Look for example at the source to libcdio's command-line interface, cd-paranoia.c, and you'll see it has code to swap bytes based on processor behavior. 

If you have libcdio installed, a simple way to determine if libcdio's paranoia library is reporting the wrong endianness is to use its command-line interface which is by default installed under the name cd-paranoia. I think the verbose option will show what it thinks the drive is. If that gets the wrong answer or cd-paranoia doesn't work without specifying an option, then the problem is in the libcdio's drive detection. (And if it get's the wrong answer, I'd be curious to know if there is a Solaris port of cdparanoia which get's the *right* answer.)

Determinining the drive endianness is based on cdparanoia's hueristic. It sometimes gets this wrong. And the wrongness could be in an OS-specific part of the library. That is why both the cdparanoia command-line program and libcdio's port of this have an option to explicitly specify the drive endianness. 

A plugin should also provide an option or configuration setting for overriding this too.


(In reply to comment #3)
> > However, I think libcdio should only handle the endianness
> > of CDROM, and should leave gstreamer to handle the endianness of different
> > processor. 
> 
> That's how it works at the moment. GStreamer currently assumes that the data it
> gets from libcdio is in native endianness and the output caps will reflect
> this.
> 
> So either that assumption is generally wrong (couldn't find anything to that
> effect in the docs and haven't seen any complaints from Linux/PPC users yet
> either), or the endianness of the data varies depending on the drive. In the
> first case, we need to fix things in GStreamer's cdiocddasrc element, in the
> second case we need libcdio API to provide us with the information about
> drive/data endianness so we can swap the endianness as appropriate or set the
> correct output caps.
> 

Comment 5 Tim-Philipp Müller 2006-11-20 11:29:20 UTC
Rocky:

Thanks for the fast reply. What I am really looking for, however, is a solution  that doesn't involve any of the cdparanoia API, ie. a libcdio.so-based solution (for various reasons). Is there anything like that?

If what you say about drives is true, maybe we should just assume that data is always little endian (on all architectures), and provide a way to override this for the few cases where this is not the case. Should probably also check with PPC folks to see how things are working there at the moment.

Comment 6 rocky 2006-11-20 13:12:21 UTC
Personally, I think assuming a drive is Little Endian and having a way for a user to override this would be simple, fast (because no data sampling is needed), and would cover (I believe) 99% of the cases. Anecdotal evidence is that it is only older (Sony?) drives (and possibly drives on Solaris boxes) that were ever Big Endian. 

As for the API, I don't understand what you are asking for or why. libcdio took cdparanoia library's API and removed some things that it couldn't support (and shouldn't have been exposed and are even slated to go away in future releases of cdparanoia). It is good to keep compatibility with cdparanoia since that's what was previously used and what people programmed to. Why create another API that does the same thing as something that's already out there and is being used without dissent.

What was done was just to replace native GNU/Linux file and ioctl access (the original source only works on GNU/Linux) with the corresponding libcdio routines. The libcdio library, libcdio_paranoia, corresponds to cdparanoia's libparanoia. However libcdio_paranoia is completely separate and does not depend on libparanoia, even if they implement roughly the same API and in fact share a lot of the same source code.
Comment 7 rocky 2006-11-20 16:34:44 UTC
How do you know that the Sparc CD-ROM is little endian? And what does the command-line tool cd-paranoia report the drive is? 

(In reply to comment #0)
> Please describe the problem:
> When play CD with Sound-juicer with a Sparc box, the output is totally garbled
> noise. 
> 
> Steps to reproduce:
> 1. Place a CD into CDROM and launch Sound-juicer
> 2. Press play button
> 
> 
> 
> Actual results:
> The garbled noise outputed to the speaker
> 
> Expected results:
> The music should be played
> 
> Does this happen every time?
> yes
> 
> Other information:
> I figured out that the CDROM with Sparc box is little-endian, while sparc is
> big-endian, and gstreamer doesn't consider about this endianness issue. I
> suggest the gstcdiocddasrc.c in gst-plugin-good/ext/cdio directory should take
> care of this.
> 

Comment 8 Chris Wang 2006-11-21 02:38:54 UTC
I run the paranoia which included in libcdio's example directory, and it reported 
the CDROM on Sparc is little-endian. And I do think it make sense since the Sparc processor is big endian and thus why the CD cannot be played properly. Assume the CDROM on Sparc box is Big-endian and Sparc processor is Big-endian. then it will not has this problem
Comment 9 rocky 2006-11-21 03:52:55 UTC
Tim-Philipp Müller; 

Comment #8 drives home the point of why the best thing in the long run is just to have a setting to force drive endianness with instructions: "if one setting doesn't work, then try one of the others". (The 3 settings could be something like "auto" "type 1" and "type 2". Calling the CD-ROM characteristic "endianness", while technically correct just seems to cause lots of confusion.

For the record, I have a Solaris SPARC box which cd-paranoia reports as little endian and cd-paranoia works fine. The drive I have in my Solaris box is
from vendor LG,  model: CD-ROM CRD-8322B, and revison 1.05.

I tried looking at cdiocddasrc http://webcvs.freedesktop.org/gstreamer/gst-plugins-good/ext/cdio/ but I don't see the paranoia part let alone the part where the CPU endianness is taken into account (as it is in libcdio/src/cd-paranoia/cd-paranoia.c). But I could have easily missed this or more likely it's elsewhere.

Chris Wang:

You originally wrote:
  I figured out that the CDROM with Sparc box is little-endian,

Again, how did you determine this? 

Also, please try cd-paranoia from libcdio and rip one of the tracks and see if that works. And finally run cd-drive and send back what the vendor, drive and model are as I've done above. 

Your views on how to fix things have been noted. However more helpful would be getting and giving the information requested. Thanks.


Comment 10 Chris Wang 2006-11-21 04:05:34 UTC
I believe the cdparanonia works fine with Sparc box as it did took care of the Endianness issue as you can tell from the source code.  However, when you use Sound-juicer, it will invoke cdiocddasrc to read the CD (cdiocddasrc->cddabasesrc->solaris.c from libcdio). I don't think any of them processed endianness. 

I do see there is a directory in Gst-plugin-base/ext/ called cdparanonia. but I don't see this have been compiled in my case. I think may be to use cdparanonia to  replace cdiocddasrc will use the paranonia API and then the CDROM's endianness can be determined 
Comment 11 Tim-Philipp Müller 2006-11-21 11:45:37 UTC
> I tried looking at cdiocddasrc
> http://webcvs.freedesktop.org/gstreamer/gst-plugins-good/ext/cdio/ but I don't
> see the paranoia part let alone the part where the CPU endianness is taken 
> into account (as it is in libcdio/src/cd-paranoia/cd-paranoia.c).

That's correct. The cdio plugin doesn't use any of the cdparanoia API. That's partly because it has been written before that API made it into libcdio. That's also why I was asking for a solution to this problem without involving the cd-paranoia API.

The CPU endianness isn't "taken into account" in the code, it is simply assumed. The assumption was simply that cdio delivers samples in cpu endianness (made in the absence of any hints that this might not be so). I see now that this assumption is incorrect.

From your comments and the header files I conclude then that when reading audio using the 'raw' cdio API, the endianness is undefined and there is no API to find out either, nor will there be. This means we'll have to up the cdio requirements and use cdio-cdparanoia. That's fine with me, I was just trying to solve this without involving cdio-cdparanoia if possible first (since up'ing the requirements obviously means the plugin won't be available any longer on certain older distributions).
Comment 12 rocky 2006-11-22 00:04:12 UTC
Sorry it took so long - now I think I'm beginning to understand what you are asking for.

If you look at any various CD-DA plugins, e.g vlc or xine you'll see that if there is any byte swapping going on it's not happenning at the layer just above CD read.  And all of those seem to work fine no matter what byte sex. In the plugins I've done for those, cdio_read_audio_sectors() seems to be all that's needed in those plugins. And a similar situation is true in the versions of the plugins I didn't write.

The problem seems to come in when one uses CD paranoia. One issue here is that it tries to *interpret* the data as 16-bit units such as to look for gaps of silence. 

Personally of course, I would prefer if people use newer versions of libcdio. So making that a *requirement* is, in my opinion, a good thing. It's not just that there's more there in later versions, but various bugs and security flaws have been fixed, and in some cases embarrasing gaps in my understanding of how things work are corrected. I think most distributions provide version 0.76 which is over a year old; 0.78.2 is the current stable release.

Honestly, the only way I can think of to work around using a newer version which has features you want is basically to write that code inside the application. Overall I think you get both a harder-to-maintain and more complex program; and the quality can also be less good. 

Finally as for the API - I'll make a note to document the behavior I mention above after I verify that this is indeed what I write is the case.
Comment 13 Chris Wang 2006-11-22 03:41:40 UTC
I agree that most of CD-DA plugin doing the byte swapping o the layer above CD read, and that's why I think we should do something on gst-plugin-good/ext/cdio.

To make my question more clear. I believe there are two case of byte sex we need to consider for playing audio CD, first is the endianness of CDROM, then other is the Endianness of CPU.

For the first case, cd-paranonia has function to determine the byte sex of the cdrom while cdio don't( I believe that's what Tim mentioned here). However, I do think to make the assumption that the CDROM is little-endian is acceptable as there is very tiny model of CDROM is not.

For the second case, I think is what Rocky mentioned in Comment #12, the layer above CD read should take case of the byte sex of CPU which xine and vlc do. However, I don't see the cdio plugin from gstreamer doing anything about it, which I believe should be fixed.

I think the tiny change we can make is that keep on using cdio (do not import cd-paranonia), stay on the assumption that all CD_ROM are little endian. In gst-plugin-good/ext/cdio/gstcdiocddasrc.c function gst_cdio_cdda_src_read_sector add one routine to determine the byte sex of the processor, then swap the byte order if necessary. I have this patch on hand. and in my patch, I also add #ifdef SOLARIS to secure that the patch has no infection on other platform.

any comments?
Comment 14 rocky 2006-11-22 10:29:45 UTC
Chris Wang wrote in Comment #13
  I agree that most of CD-DA plugin doing the byte swapping o the layer above CD
read, 

I don't know who you are agreeing with, but that's *not* what I wrote. 

  CD read should take case of the byte sex of CPU which xine and vlc do.

I believe both xine and vlc and other media players *don't* do any byte swapping in when *not* using cdparanoia. I don't see this code in any CD-DA "access" module of vlc or any CD-DA  "input" plugin for xine. 

Try this test and report back the results  Take an audio CD and run the "cd-read" program from libcdio using the option --mode=audio. Do this on both the Solaris computer you are having problems with (a little endian drive on a big endian CPU) and some other computer that has a little endian drive on a little endian CPU. If you get results like I just did, you'll see you get the *same* data, possibly shifted by some sort of offset. There is no byte swapping going on in the way the data is getting passed back from the CD-ROM.

What I said was that if the data gets passed back via a cdparanoia read -- libcdio or not -- you will have to take into account the byte sex of the CPU.
Comment 15 Chris Wang 2006-11-23 01:36:58 UTC
Oh, I may forgot to mention that this is a Sparc only bug. Everything works fine in Solaris X86 for the code, and when I either swap the byte sex or force to Big endian(cd-paranonia) on Sparc, the problem is gone.

According to your comment, I think I can try to compile Xine on Solaris sparc to see if it works there. 
Comment 16 Chris Wang 2006-11-30 10:32:58 UTC
According to the sound-juicer source code, the pipeline combined by following elements cd-source, queue,audioconvert, audioresampler, volume, and gconfaudiosink. 
I believe the two possible place to fix are cd-source (cdiocddasrc) and audioconvert. I still think the best place to fix is in cdiocddasrc

FYI, I use gst-launch cdiocddasrc track=1 device=/dev/dsk/c0t2d0s2 ! sunaudiosink
to play CD, and still the music play noisy sound on Sparc, and play well on X86.

Any comments?
Comment 17 Chris Wang 2006-12-01 09:32:05 UTC
Created attachment 77475 [details] [review]
a possible patch

THis patch only solve the problem for Sparc box, I don't want to affect other platform. In this patch, I didn't malloc any new memory to avoid the memory leak
Comment 18 Brian Cameron 2006-12-01 20:43:43 UTC
Chris - a few questions.  It looks like this patch fixes the situation where the endianness of the processor and CD-drive is different (as on your machine).  It looks like this patch will reverse the endianness when programs are run on *any* SPARC machine (where endian is BIG_ENDIAN) running Solaris.

However are you sure that this is always true?  Are there some CD players that user might install into a SPARC machine where the endian-ness of the CD player would be the same.  If so, then wouldn't your patch *break* the audio playback for these users?

Do we need to check in the patch for the endianness of the CD player and only reverse the endianness on-the-fly if the endianness of the processor and CD player are, in fact, different?





Comment 19 rocky 2006-12-01 21:12:54 UTC
Exactly. 

Chris Wang seems challenged when given a request and following it. I asked how he knew his drive was little endian in comments #7 and #9. Lots of other verbiage, but nothing on that. In comment #14 I asked him to verify with cd-read whether he gets the same order between a big endian CPU and a little endian one. Again, more comments and opinions but not what was asked of him. 

My own tests on Solaris Sparc don't seem to match his, although I haven't tried specifically with gstreamer. But as a reliable source for information for what his specific situation is or what his setup is, I'm a little bit hesitant. And thus for that same reason his conclusions seem a little suspect too. See comment #10 where cdparanoia is mentioned which may or (more likely) may not be in use.

One could wait for someone else to have the problem and get more information from that.

And as I wrote in comment #14, if it is the case gstreamer has a bug, given the way it's solved in other media players, the bug may not be in cdiocddasrc, but elsewhere. ANd if I were involved in this I'd probably would want to determine this too.
Comment 20 Chris Wang 2006-12-02 06:37:03 UTC
Rocky
I think I answsered your question in comment 8 on how do I figured out my CD_ROM is little endian. On responding to your comment 14, There definitly has no different from the data pass back from CDROM as both CDROM are LITTLE-ENDIAN, but the byte sex of the CPU are different, and I believe that's the problem lied on.

I don't see the code of xine or other CD ripper, I doubt that they may process the byte sex issue on upper layer ( but I strong believe they processed it). As I mentioned on comment 16, those elements are invovled in CDplay, and I think the best place to put these endianness detection should be in cdiocddasrc. The architecture of Xine is different from Gstreamer, so Xine the endianness detection of Xine may not in cdio read layer.

Brain, in Tim's comment 11, he explained that libcdio don't have the API to determine the byte sex of the CD ROM. I agree that to put a module that can determine the it make the patch perfect, however, I think more work may need to do on it. Currently, my patch make a assumption that all CDROM are little endian.

Rocky, I have tested on many modal(Sunblade 1500,2500 etc) of Sparc box and all have little-endian CD ROM and big endian processor, and all of them cannot play CD correct with gstreamer...

I can be wrong, and there may be some Sparc box has big endian CDROM, Can you tell me your Sparc box configuration. 
Comment 21 Brian Cameron 2006-12-12 01:06:15 UTC
We talked with our HAL expert Artem Kachitchkine at Sun about this.  He has the following to say.  We were asking him if HAL could tell us whether the drive is big or little endian.

This seems like something HAL could present as a property, yes.
It appears that "big endian" drives are usually older models. All recent 
models should return little endian PCM data, regardless of the CPU endianness.

It also appears that there is no sure way to determine endianness, it looks more like guesswork. From cdda2wav README:

    I try to write correct wav files (little endian samples), but some
    cd-writers might swap them, which would result in sort of white noise
    instead of the original sounds. Cdda2wav has an endianness detector
    for these cases, but as for all automatics, it might fail on bizarre 
    samples.
    Hint: Cdda2wav can be forced to use a given input endianness with the
    -C option (arguments are 'little', 'big' or 'guess').

The corresponding guessing code is here:

http://src.opensolaris.org/source/xref/sfw/usr/src/cmd/cdrtools/cdrtools-2.01.01/cdda2wav/interface.c#284

It looks at MMC code and INQUIRY data retrieved by sending USCSI commands to the drive.

If someone is willing to write and test some standalone endianness-guessing routines, I would gladly help that someone integrate them in HAL. 

----

My (Brian Cameron) comment about this is that we should probably not apply this patch since by default we should support recent hardware and not old hardware.  
Perhaps if we don't fix this via HAL, we could add some configuration to allow 
users with old drives to set up their configuration to flip the bits to make it
work for them.

Comment 22 rocky 2006-12-12 11:23:07 UTC
Are you sure that the cdda2wav code cited is the *only* part that determines endianness? cdparanoia has code like this but it has other more general code as well.

What cdparanoia also does is read the beginning audio tracks (recall that this is *only* a problem with Philips Red book or CD audio). From that it looks for the least "noise", trying both byte-order interpretations. Noise is determined by wild oscillations of the data -- if one were view the sound on an oscilloscope, the sound noise would have lots of short large spikes; "normal" sound oscillations would be smaller and smoother. And complete silence would be more or less a straight horizontal line. In libcdio the code for this is in lib/cdda_interface/common_interface.c in routine data_bigendianp. But in reality this code comes directly from cdparanoia. 
Comment 23 rocky 2006-12-13 03:58:48 UTC
The following probably goes without saying, but I say it. The cdparanoia approach for determining drive endianness has an advantage that is possibly an obstacle that HAL  would need to overcome were it to try the same thing: in cdparanoia there is a very high likelihood that will be a CDDA-format CD in the drive at the time one wants to determine endianness. 

However once the drive endianness is determined HAL (or cdparanoia) could save this information filed under information for that model vendor and hardware-revision number. (Currently cdparanoia/libcdio doesn't do this. It would however be a smart thing to do.)
Comment 24 Chris Wang 2007-01-16 02:38:57 UTC
Created attachment 80355 [details] [review]
modified patch

Add getenv to give the user option to switch off this patch if one's CDROM is big endian.
Comment 25 Wim Taymans 2007-03-02 11:28:25 UTC
I was thinking about moving this logic to the base class. We also need some config parameter in the base class to signal that byte swapping might be needed. Some libraries might byteswap themselves, others might not. 
Comment 26 rocky 2007-03-02 16:08:35 UTC
Okay, but be careful here. The issue is not about (just) CPU byte swapping but CD-ROM drive byte swapping. There is a further complication in that depending on the code you use there may be something about whether CD-ROM byte swapping matches CPU byte swapping. 

It is very common for people to confuse CPU byte swapping (or Endianness) with CD-ROM drive byte-swapping/Endianness. You see come up in this thread alone.
Comment 27 Brian Cameron 2007-03-08 04:29:13 UTC
Yes, Wim, I don't think that this is the same endian-ness issue that you are discussing.

This patch fixes a bug caused when machine endianness does not match drive endianness.  All CD drives made today are little endian so the problem only shows up when running on a big-endian machine using a little-endian CD drive. 

Some very old CD drives were big endian so I guess you'ld also see the problem running on a little-endian machine with a very old drive.

I think it is okay to just not support older big-endian CD drives and tell people they need to buy a new CD drive if they want to play CD audio.  CD drives are not expensive these days.

Currently GStreamer does not work at all on Sparc for 99.9% of all users who have the common little-endian drives.  I'd recommend applying this patch so that CD audio playback is functional on Sparc.

That said, I would recommend making a minor modification to the patch so that instead of using #ifdef SOLARIS you instead use 

#ifdef __BIG_ENDIAN__

so that this functionality works properly on other big endian machines, not just Sparc.

Comment 28 Tim-Philipp Müller 2007-03-08 09:21:33 UTC
> #ifdef __BIG_ENDIAN__
> so that this functionality works properly on other big endian machines, not
> just Sparc.

This, however, would break things on Linux/PPC, wouldn't it? (given that it seems to work fine there now)
Comment 29 Brian Cameron 2007-03-09 09:50:31 UTC
It surprises me that other BIG_ENDIAN processors don't have the same problem.  Any ideas why?
Comment 30 Wim Taymans 2007-03-09 10:13:53 UTC
My problem with patching this is twofold:

 - which library does the byte swapping to native order internally and on what 
   platforms (this seems to be the case on PPC but not on sparc, dunno)
 - It would be nice to add this functionality in the base class, maybe based on
   a setting by the subclass
Comment 31 rocky 2007-03-09 10:42:27 UTC
(In reply to comment #29)
> It surprises me that other BIG_ENDIAN processors don't have the same problem. 
> Any ideas why?
> 

If this is the case and I guess it is, then it suggests there's a bug in libcdio's solaris driver. But if there were a bug in the solaris driver, one should also see the difference in bytes when using cd-read. By the way, looking at cd-read output was requested in comment #14 above.

In the past, when I've run cd-read output and compared with other OS's and drives, I've noticed a slightly different offset ordering with cd-read (of something less than a sector read), but not a different byte swapping ordering. Of course I just have one computer that I use for this test.

On thing that has that's bothered me about the patch and this thread is that it just strikes me that patching where suggested here is the wrong place one way or another. 

I've mentioned above the possibility that this could be a libcdio solaris driver bug, and the fix then would be in that driver.

At the other end on the user interface side, I think there should be some "advanced" setting to swap for audio. That covers those cases mentioned above in comment #27 that slip by. (It seems a little self serving for a person who works for a company that sells hardware to suggest that people who have this problem buy newer hardware - especially given there's a simple software fix for this.) It also handles the case where cdparanoia can't automatically determine the drive endiannes. (Cdparanois is not used in this situatinon but one day it might.) The cdparanoia algorithm may get the wrong endianness if the audio CD started out immediately with something that's close to noise. And note that the standalone cdparanoia/cd-paranoia programs do have an option to force the drive endianness interpretation.
Comment 32 Brian Cameron 2007-03-22 13:51:11 UTC
Note on Solaris, we plan on removing libcdio anyway since it is GPL and we really shouldn't use it in GStreamer.  We plan to move to using an ioctl based CDDA plugin discussed in bug #413705.  

Therefore it might make sense just to close this bug as willnotfix.
Comment 33 rocky 2007-04-20 10:12:39 UTC
(In reply to comment #32)
> Note on Solaris, we plan on removing libcdio anyway since it is GPL and we
> really shouldn't use it in GStreamer.  We plan to move to using an ioctl based
> CDDA plugin discussed in bug #413705.  
> 
> Therefore it might make sense just to close this bug as willnotfix.
> 

Although it may be a pity that Sun Microsystems for whatever reason can't deal with GPL here, I am at a loss to understand how Sun/Solaris's decision changes what Gnome/Gstreamer developers work on. They are independent organizations, right? 

I don't know if Brian is a Gnome/Gstreamer developer - I haven't seen anything indicating this - but I can say with certainty he's not a libcdio developer. And I've indicated that if the bug can be identified as a libcdio bug, we'll probably fix it (eventually).

So I find it odd, if not also again a bit self-serving, for him to suggest Gstreamer policy. 
Comment 34 Edward Hervey 2007-04-20 10:18:41 UTC
rocky: it's just that we don't allow GPL-licensed code in gst-plugins-good/-base , it will go in -ugly. And it's not Brian decding, but a decision that GStreamer developers made almost 2 years ago.

More info here : http://gstreamer.freedesktop.org/documentation/splitup.html
Comment 35 rocky 2007-04-20 10:48:53 UTC
This bug, 377280, is about drive endianness - not the good, bad, and ugly. There's another "bug" and discussion about what goes where. Although it may be the case that bugs in good plugins are fixed with a higher priority, reading your above link I get the impression that ugly ones are too.
Comment 36 Brian Cameron 2007-04-23 05:36:12 UTC
Rocky - 

Sun Microsystems can deal with GPL licenses just fine.  We just do not like to link GPL libraries into LGPL libraries (such as GStreamer).  Since the GStreamer libcdio cdda plugin was the only place libcdio was ever used on Solaris, there is no longer a need for libcdio on Solaris now that we have stopped building this particular plugin.

I have been involved with working on GStreamer since 0.6.  Most of my efforts have been focused on the SunAudio code in gst-plugins-good and getting GStreamer to build and work on Solaris, but I have been involved with other areas of GStreamer from time-to-time.  Including helping with legal issues, sorting out licensing issues, and such.  While I may not be a "core" GStreamer developer, I'd say most of the GStreamer team knows who I am.

I thought in discussion of this bug we decided that this *is not* a GSTreamer libcdio cdda plugin bug, and is instead an issue in libcdio directly.

Therefore, my suggestion to remove this bug as WILLNOTFIX is that this is not a GNOME or GSTreamer bug.  The bug should probably be moved to whatever bug tracking tool the libcdio community uses, no?

The fact that libcdio is no longer shipped with Solaris lowers the priority of fixing Solaris specific bugs in libcdio, but as you say, I wouldn't be surprised if this issue eventually gets fixed by someone in the free software community.
Comment 37 rocky 2007-04-23 11:16:40 UTC
(In reply to comment #36)
> Rocky - 
> 
 [Stuff not relevant to this bug deleted]
> 
> 
> I thought in discussion of this bug we decided that this *is not* a GSTreamer
> libcdio cdda plugin bug, and is instead an issue in libcdio directly.

The status of this bug is marked "need info" and that is appropriate. 

> 
> Therefore, my suggestion to remove this bug as WILLNOTFIX is that this is not a
> GNOME or GSTreamer bug.  The bug should probably be moved to whatever bug
> tracking tool the libcdio community uses, no?

No. It has not been demonstrated to be a libcdio bug. The last part of comment #14 describes what steps are needed to show this is a libcdio bug. And as late as comment #31 I reiterated why I thought this might not a libcdio bug. 

Furthermore, since this is difficult to address totally reliably and completely automatically in the general case, it may be desireable (if not also simple) to add an override option in the plugin and/or front end. This would be in gstreamer or the cdiocddasrc plugin.

> 
> The fact that libcdio is no longer shipped with Solaris lowers the priority of
> fixing Solaris specific bugs in libcdio

To be precise, it may lower the priority of *your* fixing Solaris bugs in libcdio and/or cdiocddasrc (although to date I don't recall any patches to the libcdio part from Brian Cameron - I'm pretty good about noting contributions). 

However it's possible, even likely, that there are others for which none of this matters. Clearly if folks cared about what gets shipped with Solaris, there is much software that wouldn't have been written. (Possibly even stuff that Sun *does* eventually ship which it did not originally write). Similarly the same is true with Gstreamer plugins or else there wouldn't be all of those other plugins outside the "good" category. And I suspect bugs in those other categories are reported via this mechanism and get fixed.

The bottom line is that folks work on what they want to. Gstreamer policy may *suggest* what's more important but it doesn't dictate it. So what I take offense here is even the suggestion that since it's not of interest to you and/or Sun, or that it will no longer be in the "good" category, that this should be be marked "will not fix" and/or not of interest to other Gstreamer developers. 

, but as you say, I wouldn't be
> surprised if this issue eventually gets fixed by someone in the free software
> community.

which includes Gstreamer community ;-)


Comment 38 Tim-Philipp Müller 2007-04-23 15:09:08 UTC
> Therefore, my suggestion to remove this bug as WILLNOTFIX is that this is not a
> GNOME or GStreamer bug.

Since the signal-to-noise ratio of this bug is pluging into unknown depths anyway, allow me to point out that it was initially suggested that this bug be closed as WONTFIX because Sun will be moving from the libcdio-based plugin to a different one, not because it has been established that this is a libcdio bug, but anyway ;)


This bug hasn't been closed yet, because if something doesn't work with our current cdiocddasrc plugin there's a bug here somewhere, and that should still be looked into, whether Sun intends to use this particular plugin or not.


This is also independent of whether we want to add an override to specify the drive endianness or not (we might want to do that, but it's not really a proper solution).


Maybe there's a way we can resolve the questions raised in comment #14?

Rocky: are there some straight forward commands (like, e.g., 'cd-read --mode=audio --number=1000 | gzip > log1.gz') that Chris or someone else could run on different machines and where the output logs would be useful for you if they were attached to this bug report? (apologies for the awful grammer, you know what I mean)

Comment 39 rocky 2007-04-24 00:34:10 UTC
Created attachment 86887 [details]
Log of libcdio working properly on GNU/Linux versus Solaris. Suggests how to look for a libcdio bug
Comment 40 rocky 2007-04-24 05:24:09 UTC
> 
> Rocky: are there some straight forward commands (like, e.g., 'cd-read
> --mode=audio --number=1000 | gzip > log1.gz') that Chris or someone else could
> run on different machines and where the output logs would be useful for you if
> they were attached to this bug report? (apologies for the awful grammer, you
> know what I mean)

The process I used is this. I'll also attach an abbreviated log of the output.

1. Make sure you have an audio cd installed. The command cd-info will verify you this (just so there's no confusion here). It will also give you track start information. That is:

   cd-info -A --no-device-info

You should get output like this:

Disc mode is listed as: CD-DA
CD-ROM Track List (1 - 22)
  #: MSF       LSN    Type   Green? Copy? Channels Premphasis?
  1: 00:02:00  000000 audio  false  no    2        no
  2: 02:28:45  010995 audio  false  no    2        no
               ^^^^^^^^^^^^

2. Run cd-read to dump a small bit of audio info. I usually choose track 2.

  cd-read --mode=audio --start=10995 --number=1

You'll see output something like this:
0x0000: eeff 78ff eeff 74ff f4ff 74ff f0ff 76ff   ..x...t...t...v.
0x0010: efff 72ff f0ff 74ff e9ff 78ff e8ff 73ff   ..r...t...x...s.
0x0020: e8ff 75ff eaff 74ff ebff 74ff f0ff 74ff   ..u...t...t...t.
0x0030: efff 72ff e9ff 72ff ebff 72ff e3ff 72ff   ..r...r...r...r.

Eject the audio CD. Let's say the above was on a Solaris box where things aren't working. So now run it on a computer where things are working. In my situation this is what I get from GNU/Linux.

Here's the corresponding output:

0x0000: 2100 b4ff 1f00 b3ff 1e00 b1ff 1f00 b2ff   !...............
...
0x0120: eeff 78ff eeff 74ff f4ff 74ff f0ff 76ff   ..x...t...t...v.
0x0130: efff 72ff f0ff 74ff e9ff 78ff e8ff 73ff   ..r...t...x...s.
0x0140: e8ff 75ff eaff 74ff ebff 74ff f0ff 74ff   ..u...t...t...t.
0x0150: efff 72ff e9ff 72ff ebff 72ff e3ff 72ff   ..r...r...r...r.
0x0160: e7ff 6fff e2ff 6cff dbff 69ff deff 65ff   ..o...l...i...e.
0x0170: e2ff 69ff dcff 68ff dcff 6eff dbff 6bff   ..i...h...n...k.
0x0180: d7ff 6bff daff 69ff d5ff 6bff d9ff 6eff   ..k...i...k...n.
0x0190: d6ff 71ff d3ff 6eff d6ff 72ff d5ff 72ff   ..q...n...r...r.

Note that on GNU/Linux things have been shifted by 0x120. But the important part is that the data is the same: 72ff f0ff remands that rather than ff72 fff0
Comment 41 Chris Wang 2007-05-14 09:52:57 UTC
I think Rocky's report shows this is not a bug in libcdio, what cd-read do is invoke cdio_read_sector to read from the CDROM, pass back the data to a uint8 buffer, and print out the data in hex decimal. 

The test report indicate that the data pass back from both (little-endian) CD_ROM are correct, but remember, these are Little Endian data. And I think the libcdio do 
what it should do and with no error so far.

In our next step, these raw data are pass to gstreamer( or some other application) to play. Since the AudioCD is 16bits/sample, and the data are in little-endian. So there is problem in big-endian machine( imagine we pass the data 1, but the cpu think it was a 256), and that's why I made the patch in gstcdiocdda.c


 
Comment 42 rocky 2007-05-14 15:28:17 UTC
I'm not sure I understand Chris Wang. My "report" shows there is no problem with libcdio on my Solaris Ultra computer. If what Chris means is that he tried the same thing on his Solaris box where he noticed a problem and Chris got roughly the same results then, yes, I guess there is no bug in libcdio for this. 

However if the problem is not in libcdio but a CPU endian problem, then you'll see this on *any* big-endian CPU like Power, PowerPC (e.g. G4's and G5's) Sparc,  UltraSparc, MIPS, Alpha, etc. And this has nothing to do with the Solaris OS.

And even if that is the situation, again I doubt the proper fix is here but somewhere in the part that handles treating data as a 16-bit sample. 
Comment 43 Chris Wang 2007-05-15 02:49:44 UTC
Rocky:
You are right, I got the same result from my Sparc box. I don't have environment of other big-endian machine, can someone verify it on them?

However, I'm not sure if we should put the fix in the place where we handle the 16bit data, as I think most decoder(eg. mp3) will handle the byte-order problem before they pass the decoded raw data to resampler. 


This just my two cents
Comment 44 rocky 2007-05-15 03:18:37 UTC
Chris: Thanks for the information. 

What we need to now understand is whether this is seen on other Big Endian computers. Given the information you report it should be. And because you get the same results on Solaris (other than data offset which is to be expected with that reading mode) as on some other OSes, there's nothing Solaris-specific about this, and has nothing to do with CD-drive endianness. 

If it is the case that one gets the garbled audio results on other Big-Endian computers, then the patch should not refer to Solaris or to CD-drive endianness (that getenv in the patch) but rather CPU endianness. That is, patch then should be changed a little bit.


Comment 45 Tim-Philipp Müller 2008-08-12 20:19:52 UTC
cdiocddasrc has moved from -good to -ugly, updating component.
Comment 46 Tim-Philipp Müller 2012-11-27 18:21:01 UTC
commit 1ab41f83b74bb3c1390083e35b2bf1af5e8d7144
Author: Tim-Philipp Müller <tim@centricular.net>
Date:   Tue Nov 27 17:07:31 2012 +0000

    cdiocddasrc: detect whether drive produces samples in non-host endianness
    
    If drive produces samples in other endianness than the host,
    we need to byte swap them before pushing them out, or we
    produce nothing but noise. cdparanoia detects this automatically,
    but libcdio does not, so we have to do it ourselves.
    
    This is needed on e.g. the PowerBook G4 with Matshita UJ-816 drive.
    
    https://bugzilla.gnome.org/show_bug.cgi?id=377280