Re: [B2-devel] bin/cue files

[Geoffrey B. <geo...@in...>]

I think I've made all the changes except one -- I didn't change the
free/malloc pair
for realloc.  I think the error cases are easier to understand for
free/malloc than
for realloc and there isn't a performance issue since an allocation is
expected only when a stream is
started.  The following should work:

1) Basilisk and SheepShaver with sdl-audio and bincue on linux and os x
2) SheepShaver with bincue and core audio on os x -- the Basilisk
configuration seems to be lagging Sheepshaver and doesn't support core
audio even without the bincue support.

I haven't done the changes necessary for windows and would have
to set up a system to do them.

Geoffrey

On Mon, Oct 4, 2010 at 7:32 PM, Alexei Svitkine
<ale...@gm...> wrote:
> One more thing I forgot to mention:
>
> Please put the license comment first - above your comment explaining
> the file. Begin the license comment with the name of the file and a
> one-line description of what it does (see how other files do it).
> Also, include your name in the Copyright in the license comment
> (unless you want your changes under some other license).
>
> After this, you can put the other comment that explains the file in
> more depth (limitations, etc) following the license comment.
>
> Thanks again,
>
> -Alexei
>
> On Mon, Oct 4, 2010 at 7:19 PM, Alexei Svitkine
> <ale...@gm...> wrote:
>> Hi Geoffrey,
>>
>> Patch looks mostly good, though I have a few comments:
>>
>> 1. In LoadCueSheet(), you call fstat() without checking the return
>> value for success - please handle the failure case.
>>
>> 2. I suggest refactoring the keyword parsing part of LoadCueSheet()
>> into a separate helper function to make the code more readable. Either
>> the whole while() itself or just the loop body.
>>
>> 3. You're still using non-Sheepshaver style braces / incorrect
>> indentation in fill_buffer(). In that block, I also suggest using
>> realloc() instead of free()/malloc() (and don't forget to free the
>> original buf if realloc() fails).
>>
>> 4. Can you add a comment explaining why the #ifndef SHEEPSHAVER block
>> is necessary?
>>
>> 5. Can you explain the logic in read_bincue()? It seems you're reading
>> by RAW_SECTOR_SIZE, but incrementing bytes_read / decrementing len by
>> 'available'. This seems incorrect to me, but I'm not familiar with the
>> format.
>>
>> 6. Can you make the first if in CDPlay_bincue consistent with the
>> similar ones in other functions (i.e. remove the extra parens)?
>>
>> 7. You're not checking the return value of lseek() in a couple of
>> places. Likewise with malloc()/strdup().
>>
>> 8. I suggest using sprintf() in the "FILE" keyword handling section
>> instead of strcpy()+strcat()+srcat().
>>
>> 9. Can there be multiple "FILE" keywords? If so, you'll be leaking
>> cs->binfile strings.
>>
>> 10. Please add D(bug()) lines in all the places in the keyword parsing
>> code where you goto fail.
>>
>> Please address the above comments and re-send the bincue_unix.cpp file. Thanks!
>>
>> -Alexei
>>
>> On Mon, Oct 4, 2010 at 9:36 AM, Geoffrey Brown <geo...@in...> wrote:
>>> Here's a second attempt.  I added the appropriate license -- in reviewing my
>>> code it appears only dosbox contributed any code and this could easily
>>> be excised (though it uses the same gpl version as sheepshaver/basilisk).
>>>
>>> I've attempted to follow the existing brace style and made sure tabs are
>>> used instead of spaces.
>>>
>>> I've also made fixes 3 & 4.
>>>
>>> Geoffrey
>>>
>>> On Sat, Oct 2, 2010 at 11:40 AM, Alexei Svitkine
>>> <ale...@gm...> wrote:
>>>> Some comments:
>>>>
>>>> 1.
>>>>
>>>> "Includes ideas, even code fragments, from dosbox and libcdio hence
>>>> must obey those licences"
>>>>
>>>> What are their licenses?
>>>>
>>>> 2.
>>>>
>>>> Please use a consistent brace/indentation style as the rest of the
>>>> SheepShaver / Basilisk II codebases. Currently your code mixes and
>>>> matches tabs and space (the existing codebase is standardised on tabs)
>>>> and uses a different brace style then the rest of the code.
>>>>
>>>> 3.
>>>>
>>>> In audio_sdl.cpp, please include the relevant header instead of
>>>> declaring extern functions right before calling it.
>>>>
>>>> 4.
>>>>
>>>> Please update your CVS - the video_blit.cpp change already exists in trunk.
>>>>
>>>> Cheers,
>>>>
>>>> -Alexei
>>>>
>>>> On Thu, Sep 30, 2010 at 9:07 AM, Geoffrey Brown <geo...@in...> wrote:
>>>>> On Wed, Sep 29, 2010 at 9:47 PM, Alexei Svitkine
>>>>> <ale...@gm...> wrote:
>>>>>> If by svn, you mean cvs, then you've more or less got it. ;)
>>>>>>
>>>>>> I suggest passing in -u to the diff command to get a unified diff
>>>>>> (easier to read). Also, if your changes involve modification to files
>>>>>> in both Basilisk and SheepShaver trees, you need to create two
>>>>>> patches, one from each tree.
>>>>>>
>>>>>> Cheers,
>>>>>>
>>>>>> -Alexei
>>>>>>
>>>>>> On Wed, Sep 29, 2010 at 9:41 PM, Geoffrey Brown <geo...@in...> wrote:
>>>>>>> Sure.  I need a little handholding on making the diffs.
>>>>>>> Did you have in mind something like the following ?
>>>>>>>
>>>>>>> 1) I checkout a clean svn snapshot
>>>>>>> 2) make my changes
>>>>>>> 3) svn diff
>>>>>>>
>>>>>>> Geoffrey
>>>>>>>
>>>>>>> On Wed, Sep 29, 2010 at 9:25 PM, Alexei Svitkine
>>>>>>> <ale...@gm...> wrote:
>>>>>>>> Can you make a patch of the changes (using diff) rather than a tar.gz
>>>>>>>> of the files?
>>>>>>>>
>>>>>>>> -Alexei
>>>>>>>>
>>>>>>>> On Wed, Sep 29, 2010 at 9:20 AM, Geoffrey Brown <geo...@in...> wrote:
>>>>>>>>> I've developed changes to Basilisk and Sheepshaver to provide limited support
>>>>>>>>> for bin/cue files specifically to enable the use of (images of) mixed
>>>>>>>>> digital/audio cdroms.
>>>>>>>>>
>>>>>>>>> It's been suggested on the SheepShaver forum that I post those files
>>>>>>>>> here.  Is it
>>>>>>>>> appropriate to attach a tar.gz file to this list ?   If that's
>>>>>>>>> inappropriate, is there
>>>>>>>>> somebody who would like to catch these files, possibly for checkin to
>>>>>>>>> the cvs repository ?
>>>>>>>>>
>>>>>>>>> In SheepShaver, SDL audio and OS X core audio work and have been
>>>>>>>>> tested on linux and OS X.
>>>>>>>>> In Basilisk SDL audio works and has been tested on linux.
>>>>>>>>>
>>>>>>>>> The only bin/cue files my code supports are ones with a single binary
>>>>>>>>> and 2532 tracks (audio and raw
>>>>>>>>> data).  There are other limitations, but those are the key ones.
>>>>>>>>>
>>>>>>>>> Geoffrey Brown
>>>>>>>>>
>>>>>>>>> ------------------------------------------------------------------------------
>>>>>>>>> Start uncovering the many advantages of virtual appliances
>>>>>>>>> and start using them to simplify application deployment and
>>>>>>>>> accelerate your shift to cloud computing.
>>>>>>>>> http://p.sf.net/sfu/novell-sfdev2dev
>>>>>>>>> _______________________________________________
>>>>>>>>> basilisk-devel mailing list
>>>>>>>>> bas...@li...
>>>>>>>>> https://lists.sourceforge.net/lists/listinfo/basilisk-devel
>>>>>>>>>
>>>>>>>>
>>>>>>>> ------------------------------------------------------------------------------
>>>>>>>> Start uncovering the many advantages of virtual appliances
>>>>>>>> and start using them to simplify application deployment and
>>>>>>>> accelerate your shift to cloud computing.
>>>>>>>> http://p.sf.net/sfu/novell-sfdev2dev
>>>>>>>> _______________________________________________
>>>>>>>> basilisk-devel mailing list
>>>>>>>> bas...@li...
>>>>>>>> https://lists.sourceforge.net/lists/listinfo/basilisk-devel
>>>>>>>>
>>>>>>>
>>>>>>> ------------------------------------------------------------------------------
>>>>>>> Start uncovering the many advantages of virtual appliances
>>>>>>> and start using them to simplify application deployment and
>>>>>>> accelerate your shift to cloud computing.
>>>>>>> http://p.sf.net/sfu/novell-sfdev2dev
>>>>>>> _______________________________________________
>>>>>>> basilisk-devel mailing list
>>>>>>> bas...@li...
>>>>>>> https://lists.sourceforge.net/lists/listinfo/basilisk-devel
>>>>>>>
>>>>>>
>>>>>> ------------------------------------------------------------------------------
>>>>>> Start uncovering the many advantages of virtual appliances
>>>>>> and start using them to simplify application deployment and
>>>>>> accelerate your shift to cloud computing.
>>>>>> http://p.sf.net/sfu/novell-sfdev2dev
>>>>>> _______________________________________________
>>>>>> basilisk-devel mailing list
>>>>>> bas...@li...
>>>>>> https://lists.sourceforge.net/lists/listinfo/basilisk-devel
>>>>>>
>>>>>
>>>>> ------------------------------------------------------------------------------
>>>>> Start uncovering the many advantages of virtual appliances
>>>>> and start using them to simplify application deployment and
>>>>> accelerate your shift to cloud computing.
>>>>> http://p.sf.net/sfu/novell-sfdev2dev
>>>>> _______________________________________________
>>>>> basilisk-devel mailing list
>>>>> bas...@li...
>>>>> https://lists.sourceforge.net/lists/listinfo/basilisk-devel
>>>>>
>>>>>
>>>>
>>>> ------------------------------------------------------------------------------
>>>> Start uncovering the many advantages of virtual appliances
>>>> and start using them to simplify application deployment and
>>>> accelerate your shift to cloud computing.
>>>> http://p.sf.net/sfu/novell-sfdev2dev
>>>> _______________________________________________
>>>> basilisk-devel mailing list
>>>> bas...@li...
>>>> https://lists.sourceforge.net/lists/listinfo/basilisk-devel
>>>>
>>>
>>> ------------------------------------------------------------------------------
>>> Virtualization is moving to the mainstream and overtaking non-virtualized
>>> environment for deploying applications. Does it make network security
>>> easier or more difficult to achieve? Read this whitepaper to separate the
>>> two and get a better understanding.
>>> http://p.sf.net/sfu/hp-phase2-d2d
>>> _______________________________________________
>>> basilisk-devel mailing list
>>> bas...@li...
>>> https://lists.sourceforge.net/lists/listinfo/basilisk-devel
>>>
>>>
>>
>
> ------------------------------------------------------------------------------
> Virtualization is moving to the mainstream and overtaking non-virtualized
> environment for deploying applications. Does it make network security
> easier or more difficult to achieve? Read this whitepaper to separate the
> two and get a better understanding.
> http://p.sf.net/sfu/hp-phase2-d2d
> _______________________________________________
> basilisk-devel mailing list
> bas...@li...
> https://lists.sourceforge.net/lists/listinfo/basilisk-devel
>