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
