Re: [B2-devel] bin/cue files

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

Sounds interesting.

By the way, I am not against patches refactoring the current code to
make it cleaner and easier to understand.

Cheers,

-Alexei

On Tue, Oct 5, 2010 at 8:45 PM, Geoffrey Brown <geo...@in...> wrote:
> Thanks for all your help.
>
>  I've also been working on enabling the use of vhd files (especially
> snapshots) using the now open source libvhd (released by xen).  It's
> all working, but
> it took some tweaking to libvhd to compile on OS X -- I've no idea how hard
> it would be on Windows.  Again this requires changes to sys_unix.cpp.  Actually
> the hardest part was making those changes to sys_unix.cpp.  It's too
> bad that the cdrom and
> "disk" support doesn't make better use of c++ -- with the current implementation
> of sys_unix (and I imagine the windows equivalent) all the special case
> code is mixed together which makes it hard to follow.
>
> Geoffrey
>
> On Tue, Oct 5, 2010 at 8:36 PM, Alexei Svitkine
> <ale...@gm...> wrote:
>> 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
>>>
>>>
>>
>> ------------------------------------------------------------------------------
>> 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
>>
>
> ------------------------------------------------------------------------------
> 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
>