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

Reply via email to