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
--- helix.bif 2006-06-14 14:49:14.788250000 +0530
+++ helix.bif 2006-06-14 14:56:17.979194000 +0530
@@ -1810,6 +1810,7 @@
datatype_sdp
datatype_lsd
datatype_mdf_audio_common
+ datatype_tone
</dependlist>
</module>
@@ -2381,6 +2382,8 @@
datatype_aac_codec
datatype_amr_codec
datatype_amr_fileformat
+ datatype_tone_fileformat
+ datatype_tone_renderer
datatype_aac_fileformat
common_log_logobserverfile
</dependlist>
@@ -2820,6 +2823,7 @@
datatype_avi_fileformat
datatype_common_audrend
datatype_amr_fileformat
+ datatype_tone_fileformat
datatype_mp4_audio_renderer
</dependlist>
@@ -7621,6 +7625,8 @@
datatype_mp4_audio_renderer
datatype_smil_fileformat
datatype_smil_renderer
+ datatype_tone_fileformat
+ datatype_tone_renderer
</dependlist>
</module>
@@ -7652,6 +7658,8 @@
datatype_mp3_fileformat
datatype_rm_audio_codec_ra8lbr
datatype_mp3_renderer
+ datatype_tone_fileformat
+ datatype_tone_renderer
datatype_rm_audio_codec_sipro
datatype_rm_audio_codec_ra8hbr
datatype_lsd
@@ -7927,6 +7935,8 @@
datatype_image_realpix_renderer
datatype_text_realtext_fileformat
datatype_text_realtext_renderer
+ datatype_tone_fileformat
+ datatype_tone_renderer
</dependlist>
</module>
@@ -8557,6 +8567,7 @@
datatype_lsd
datatype_ram
datatype_mp4
+ datatype_tone
protocol_sdp
all_dist_components
</dependlist>
@@ -8649,6 +8660,8 @@
datatype_image_unified_renderer
datatype_text_realtext_fileformat
datatype_text_realtext_renderer
+ datatype_tone_fileformat
+ datatype_tone_renderer
</dependlist>
</module>
@@ -13701,6 +13714,8 @@
datatype_vorbis_renderer
filesystem_memory
common_auth_authmgr
+ datatype_tone_fileformat
+ datatype_tone_renderer
</dependlist>
</module>
@@ -13856,6 +13871,8 @@
mpeg4
protocol_sdp
video_site
+ datatype_tone_fileformat
+ datatype_tone_renderer
</dependlist>
</module>
@@ -13963,5 +13980,92 @@
</dependlist>
</module>
+ <!-- DATATYPE/TONE -->
+ <module id="datatype_tone" name="datatype/tone" group="core" >
+ <attribute id="no_build"/>
+
+ <dependlist>
+ filesystem_data
+ filesystem_http
+ datatype_tone_fileformat
+ audio_tonegen
+ datatype_tone_renderer
+ </dependlist>
+ </module>
+
+ <!-- AUDIO/DEVICE -->
+ <module id="audio_tonegen" name="audio/tonegen" group="core">
+ <cvs root="helix"/>
+ <includeplatforms>
+ win32
+ </includeplatforms>
+
+ <source_dependlist>
+ common_include
+ common_container
+ common_dbgtool
+ common_system
+ common_util
+ common_runtime
+ datatype_common_container
+ datatype_common_util
+ datatype_common_include
+ </source_dependlist>
+
+ </module>
+
+ <!-- DATATYPE/TONE/FILEFORMAT -->
+ <module id="datatype_tone_fileformat" name="datatype/tone/fileformat"
group="core">
+ <cvs root="helix"/>
+ <attribute id="has_version_file"/>
+
+ <includeplatforms>
+ win32
+ </includeplatforms>
+
+ <source_dependlist>
+ common_include
+ datatype_include
+ datatype_common_container
+ </source_dependlist>
+
+ <dependlist>
+ common_runtime
+ common_container
+ common_dbgtool
+ common_util
+ common_system
+ common_log_logutil
+ </dependlist>
+
+ </module>
+
+ <!-- DATATYPE/TONE/RENDERER -->
+ <module id="datatype_tone_renderer" name="datatype/tone/renderer"
group="core">
+ <cvs root="helix"/>
+ <attribute id="has_version_file"/>
+ <includeplatforms>
+ unix mac win32 symbian openwave wince
+ </includeplatforms>
+
+ <source_dependlist>
+ common_include
+ audio_tonegen
+ datatype_common_container
+ </source_dependlist>
+
+ <dependlist>
+ audio_tonegen
+ common_dbgtool
+ common_log_logutil
+ common_util
+ common_container
+ common_runtime
+ common_system
+ datatype_common_audrend
+ datatype_common_util
+ </dependlist>
+ </module>
+
</targets>
</build>
_______________________________________________
Audio-dev mailing list
[email protected]
http://lists.helixcommunity.org/mailman/listinfo/audio-dev