Thanks for provided review comments.
Changes are now checked in.
Modified by :[EMAIL PROTECTED]
Reviewed by : [EMAIL PROTECTED]
Date : 06-13-2006
Project : Tone generator plug-in seek implementation for Helix
Client DNA
At 06:19 PM 6/14/2006, Eric Hyche wrote:
Updated changes look good to me.
> -----Original Message-----
> From: Sateesh Gadamsetty [mailto:[EMAIL PROTECTED]
> Sent: Wednesday, June 14, 2006 5:26 AM
> To: Milko Boic; [EMAIL PROTECTED]; [EMAIL PROTECTED]
> Cc: [EMAIL PROTECTED];
> [EMAIL PROTECTED];
> [EMAIL PROTECTED];
> [EMAIL PROTECTED]; [email protected]
> Subject: Re: [Ribosome-dev] RE: [Audio-dev] CR: Tone
> generator plug-in seek implementation for Helix Client DNA
>
> Thanks for provided review comments.
> Inserted my comments inline.
>
> At 10:41 PM 6/13/2006, Milko Boic wrote:
>
> >HELIX_FEATURE_TONE_GENERATOR should be used to control
> inclusion of tone
> >generation feature only in filesystem/data module. Other modules
> >(datatype/tone/fileformat, datatype/tone/renderer and
> audio/tonegen) are
> >the tone-generator itself and should not be varied based on
> this macro.
> >
> >Milko
>
> Removed the HELIX_FEATURE_TONE_GENERATOR macro present in
> other modules (datatype/tone/fileformat, datatype/tone/renderer
> and audio/tonegen).
>
>
> >At 08:19 AM 6/13/2006, Eric Hyche wrote:
> >
> >>Some comments:
> >>
> >>1) When the packet is pulled off from the queue and
> >> then called with PacketReady(), is it released
> >> after calling PacketReady()? I couldn't tell from
> >> the diff...
>
> The packet was not released after
> PacketReady() call in the provided diff file.
> Updated and tested the code by releasing
> the packet ,which was pulled off from the queue.
>
>
> >>2) In the tone renderer, is there any reason for
> >> this to still be in there?
> >>
> >> + #ifdef _WIN32
> >> + #ifdef _DEBUG
> >> + TCHAR szTmp[128];
> >> + wsprintf(szTmp, TEXT("OnPostSeek timeb4 %ld
> timeAfter %ld\n"),
> >> timeBeforeSeek, timeAfterSeek);
> >> + OutputDebugString(szTmp);
> >> + #endif
> >> + #endif
>
> Removed the above section of debug message as part of tone renderer.
>
>
> >>3) In audio/tonegen/Umakefil,
> >>
> >> +if project.IsDefined("HELIX_FEATURE_TONE_GENERATOR"):
> >> + LibraryTarget("tonegenlib")
> >>
> >> this should probably have a
> >>
> >> else:
> >> EmptyTarget()
> >>
> >> otherwise if HELIX_FEATURE_TONE_GENERATOR is not defined,
> >> then will it still compile?
>
> As for Milko suggestion,removed the HELIX_FEATURE_TONE_GENERATOR
> macro in all the makefiles present in modules other then
> data filesystem.
>
>
> >>4) This should be a bit more descriptive:
> >>
> >> + {HXLOG_TONE, "TONE"},
> >>
> >> Perhaps "Tone generator plugin"?
>
> Updated the above psz4CCDescription from "TONE" to "Tone
> generator plugin"
> in ihxtlogsystem.h.
>
> >>5) What target in helix.bif is this change in?
> >>
> >> @@ -7582,42 +7587,7 @@
> >> </source_dependlist>
> >>
> >> <dependlist>
> >> - common_runtime
> >> - common_system
> >> - common_container
> >> - common_dbgtool
> >> - common_util
> >> - common_unittest
> >> - common_auth_authmgr
> >> - common_auth_rn5auth
> >> - client_medpltfm
> >> - client_medpltfmldr
> >> - client_core
> >> - client_resource
> >> - client_xres
> >> - datatype_rm
> >> - datatype_lsd
> >> - datatype_ram_fileformat
> >> - datatype_ram_renderer
> >> - filesystem_local
> >> - filesystem_http
> >> - protocol_sdp
> >> - datatype_mp3_fileformat
> >> - datatype_mp3_renderer
> >> - video_site
> >> - datatype_mp4_fileformat
> >> - datatype_h263_renderer
> >> - datatype_mdf_video_renderer
> >> - datatype_mp4_audio_renderer
> >> - datatype_smil_fileformat
> >> - datatype_smil_renderer
> >> - datatype_i420_renderer
> >> - datatype_group_image
> >> - datatype_rm_imagemap_renderer
> >> - datatype_text_realtext_fileformat
> >> - datatype_text_realtext_renderer
> >> - mpeg4
> >> - datatype_aac_codec_fixpt_decoder
> >> + helix_client_objs
>
> The CVS diff for helix BIF was taken from old file,which
> made all the modification mentioned in 5,6,7 steps.
> Updated the helix.bif and attached the
> new diff for this bif file with this mail.
>
> Thanks
> Sateesh
>
>
> >>6) Why are these changes in here?
> >>
> >> + <!-- DATATYPE_DIST_H264_CODEC_DECODER -->
> >> + <module id="datatype_dist_h264_codec_decoder"
> >> name="datatype_dist/h264/codec/decoder" group="core"
> type="name_only">
> >> + <dependlist>
> >> + datatype_dist_h264_codec_decoder_actual
> >> + </dependlist>
> >> + </module>
> >> +
> >> + <!-- DATATYPE_DIST_H264_CODEC_DECODER_ACTUAL -->
> >> + <!-- H.264 decoder libraries -->
> >> + <module id="datatype_dist_h264_codec_decoder_actual"
> >> name="datatype_dist/h264/codec/decoder" group="core"
> >>type="distribution">
> >> + <sdk name="h264decoder_libs"
> >> path="datatype_dist/h264/codec/decoder"/>
> >> + <includeplatforms>
> >> + unix mac win32 openwave symbian wince
> >> + </includeplatforms>
> >> + </module>
> >> +
> >> +
> >>
> >>7) Why are the changes to helix_client_objs necessary?
> >>
> >> @@ -13828,55 +13826,53 @@
> >> </dependlist>
> >> </module>
> >>
> >> - <!-- helix_client_libs -->
> >> - <module id="helix_client_libs" group="core"
> name="name_only" >
> >> + <!-- helix_client_objs -->
> >> + <module id="helix_client_objs" group="core"
> name="name_only" >
> >> <includeplatforms>
> >> unix mac win32 symbian
>
> >>Thanks,
> >>
> >>Eric
> >>
> >> > -----Original Message-----
> >> > From: [EMAIL PROTECTED]
> >> > [mailto:[EMAIL PROTECTED] On Behalf Of
> >> > Sateesh Gadamsetty
> >> > Sent: Tuesday, June 13, 2006 9:08 AM
> >> > To: [EMAIL PROTECTED]
> >> > Cc: [EMAIL PROTECTED];
> >> > [EMAIL PROTECTED];
> >> > [EMAIL PROTECTED];
> >> > [EMAIL PROTECTED]; [email protected]
> >> > Subject: [Audio-dev] CR: Tone generator plug-in seek
> >> > implementation for Helix Client DNA
> >> >
> >> > Modified by :[EMAIL PROTECTED]
> >> > Reviewed by :
> >> > Date : 06-13-2006
> >> > Project : Tone generator plug-in seek implementation
> >> > for Helix Client DNA
> >> >
> >> >
> >> > Synopsis: Tone generator plug-in seek implementation for
> >> > Helix Client DNA
> >> >
> >> > Overview:
> >> > Update file format and renderer modules with seek
> >> > implementations. Then append tone sequence plug-in
> >> > modules to build system.
> >> >
> >> > Files Modified:
> >> >
> >> > 1] File system Module:
> >> >
> >> > \filesystem\data\datafsys.cpp
> >> >
> >> > 2] Tone Fileformat files:
> >> >
> >> > \datatype\tone\fileformat\toneff.cpp
> >> > \datatype\tone\fileformat\toneffdll.cpp
> >> > \datatype\tone\fileformat\pub\toneff.h
> >> > \datatype\tone\fileformat\toneffdll
> >> > \datatype\tone\fileformat\tonefflib
> >> >
> >> > 3] Tone renderer files:
> >> >
> >> > \datatype\tone\renderer\tonefmt.cpp
> >> > \datatype\tone\renderer\tonerend.cpp
> >> > \datatype\tone\renderer\pub\tonefmt.h
> >> > \datatype\tone\renderer\pub\tonerend.h
> >> > \datatype\tone\renderer\dllumakefil
> >> > \datatype\tone\renderer\libumakefil
> >> >
> >> > 4] Tone generator files:
> >> >
> >> > \audio\tonegen\tonegen.cpp
> >> > \audio\tonegen\pub\tonegen.h
> >> > \audio\tonegen\Umakefil
> >> >
> >> > 5] Common Module files:
> >> > \common\include\ihxtlogsystem.h
> >> >
> >> > 6] Ribosome Build system files
> >> > \build\bif-cvs\helix\common\build\BIF\helix.bif
> >> > \build\umakepf\helix-producer-all-defines.pf
> >> >
> >> >
> >> > Files Added:
> >> >
> >> > 7] Tone Fileformat files:
> >> > \datatype\tone\fileformat\toneffdll_win32.pcf
> >> >
> >> > 8] Common Module files:
> >> > \common\include\hxtonetype.h
> >> >
> >> > 9] Ribosome Build system files
> >> > \build\umakepf\helix-client-tone.pfi
> >> >
> >> > Newly added files are attached with this e-mail.
> >> >
> >> > Image Size and Heap Use impact (Client - Only):
> >> > Minor
> >> >
> >> > Distribution Libraries Affected:
> >> > Data File system
> >> > HTTP File system
> >> >
> >> > Distribution library impact and planned action:
> >> > None
> >> >
> >> > Platforms and Profiles Build Verified:
> >> > Windows platform, helix-client-all-defines
> >> >
> >> > Platforms and Profiles Functionality verified:
> >> > 1] Tested and verified seek functionality with splay.exe.
> >> > 2] Able to play the simple tone and polytone sequences with
> >> > multiple seek events.
> >> > 3] Tested the polytone sequences with different seek offsets
> >> > and able to handle multiple repeat events,block events and
> >> > volume change events properly.
> >> > 4] Updated the helix.bif with datatype_tone Module,which
> >> > contains the list of dependencies to build the tone
> sequence plug-in.
> >> >
> >> > Profile: helix-client-all-defines
> >> >
> >> > Branch: HEAD, Cay150
> >> >
> >> > cvs diff:
> >> >
> >> > Attached CVS diff file (filename : diff_TONE_13Jun2006.txt)
> >> > with this e-mail.
> >> >
> >> >
> >>
> >>
> >>_______________________________________________
> >>Ribosome-dev mailing list
> >>[EMAIL PROTECTED]
> >>http://lists.helixcommunity.org/mailman/listinfo/ribosome-dev
> >
>
_______________________________________________
Audio-dev mailing list
[email protected]
http://lists.helixcommunity.org/mailman/listinfo/audio-dev
