Re: [B2-devel] bin/cue files

[Alexei S. <ale...@gm...>]

I've committed your changes with some minor formatting tweaks.

I've also updated SheepShaver's root Makefile to include your two new
files in 'make links'.

Finally, I've removed the commented out ifndef SHEEPSHAVER block -
assuming it wasn't needed.

Thanks for contributing!

-Alexei

On Tue, Oct 5, 2010 at 9:46 AM, Geoffrey Brown <geo...@in...> wrote:
> 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
>>
>
> ------------------------------------------------------------------------------
> Beautiful is writing same markup. Internet Explorer 9 supports
> standards for HTML5, CSS3, SVG 1.1,  ECMAScript5, and DOM L2 & L3.
> Spend less time writing and  rewriting code and more time creating great
> experiences on the web. Be a part of the beta today.
> http://p.sf.net/sfu/beautyoftheweb
> _______________________________________________
> basilisk-devel mailing list
> bas...@li...
> https://lists.sourceforge.net/lists/listinfo/basilisk-devel
>
>