Thanks for provided review comments.
Changes are now checked in.
Modified by :sgadamsetty real.com
Reviewed by : ehychereal.com
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:sgadamsettyreal.com]
> > Sent: Wednesday, June 14, 2006 5:26 AM
> > To: Milko Boic; ehychereal.com; atlasreal.com
> > Cc: ribosome-devhelixcommunity.org;
> > common-devhelixcommunity.org;
> > filesystem-devhelixcommunity.org;
> > datatype-devhelixcommunity.org; audio-devhelixcommunity.org
> > 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: audio-dev-bounceshelixcommunity.org
> > >> > [mailto:audio-dev-bounceshelixcommunity.org] On Behalf Of
> > >> > Sateesh Gadamsetty
> > >> > Sent: Tuesday, June 13, 2006 9:08 AM
> > >> > To: atlasreal.com
> > >> > Cc: ribosome-devhelixcommunity.org;
> > >> > common-devhelixcommunity.org;
> > >> > filesystem-devhelixcommunity.org;
> > >> > datatype-devhelixcommunity.org;
audio-devhelixcommunity.org
> > >> > Subject: [Audio-dev] CR: Tone
generator plug-in seek
> > >> > implementation for Helix Client DNA
> > >> >
> > >> > Modified by :sgadamsettyreal.com
> > >> > 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
> > >>Ribosome-devhelixcommunity.org
> > >>http://lists.helixcommunity.org/mailman/listinfo/ri
bosome-dev
> > >
> >
_______________________________________________
Filesystem-dev mailing list
Filesystem-devhelixcommunity.org
http://lists.helixcommunity.org/mailman/listinfo/
filesystem-dev
|