Adddate: 2009-11-04 06:03:24
Attached New Patch against Trunk version -481
Adddate: 2009-11-05 11:49:09
Please follow following steps to build.
DM6446: ./configure --with-preset=dm6446
DM355: ./configure --with-preset=dm355
Adddate: 2009-11-05 13:19:09
Can you please clarify if this patch is meant to go into the trunk or
BRANCH_STAGING? According to Todd's post at
It looks like this is is intended for the trunk, but you mention above that
"Please review and if accepted we can apply the patch in
It looks like your patch doesn't quite comply with the patch creation process
As a courtesey, I've posted "properly_formatted_patches.zip",
which contains two example patch files. One is properly formatted for someone
that is using svn, and one is properly formatted for someone that is using git.
The filenames indicate which is which.
If you could give as much detail as possible about the design stretegy this
change is using the description, it would be much appreciated. It will help me
to understand your code changes as I go through them.
If you could clarify the above, and perhaps re-post the patch you like best
(feel free to delete the .zip file I posted). I'll can take a look.
Thanks and best regards,
Adddate: 2009-11-05 13:27:12
"If you could give as much detail as possible about the design stretegy
change is using the description, it would be much appreciated. It will help
to understand your code changes as I go through them."
What I mean is that when I open your patch the first thing I see is:
+dnl DMAI PLUGINS OPTIONS
+dnl Define presets
+ [Select a preset (autoselects the platform and plugins):
+ dm355 (dvsdk 1.30 or 2.00), dm355s (dvsdk2.0 + DM355s codecs),
+ dm357 (dvsdk 2.00), dm365 (dvsdk 2.10)
+ dm6446 (dvsdk 2.00),
+ dm6467 (dvsdk 1.40), omap35x (dvsdk 3.00)
From your description, I want to be able to tell what the code is doing, why
this change is being made, and how does this change the plugin's usage or build
process. Also, I would like to know why the changes to configure.ac were so
closely coupled to the video encode refactor that they needed to be in the same
patch. At first glance, it looks like you are enabling some functionality in
configure.ac that is beyond video encoder refactoring.
Adddate: 2009-11-06 06:13:14
Thanks a lot Don,
Sorry for my bad patch and comments.
Please find the proper patches attached. All patches need to be applied for
building the code. These patches are against
Trunk revision: 506
Note: There is no change in the functionality, its just video encoder code
- Based on the preset (which is the processor) we set the xdc target and
platform. Decide which xdm version and which codecs to enable.
- These files are the main entry point of any encoder element. Curretly Only
Video related changes added. The gstreamer element related common code is placed
here and based on the appropriate codec/xdm version a specific processing code
from the old video encoder ie gsttividenc are called.
- Holds info specific to that codec, like the pads template etc.
- removed the redundant codes that can be found in both the xdm versions.
Created a gst_tiencoder_ops structure that holds function pointers for each
encoder version. One need to fill this structure to assign the respective
functions, that will be called from the main gsttidmaienc.c file. Common info
are stored in the dmai element structure and per codec info are stored in
- All encoders will be registered through one entry point
register_dmai_encoders, No need of setting any environment variables. Based on
the preset and the configure, only allowed elements or xdm, get registered.
- modified for changes done in previous patches.
Adddate: 2009-11-08 19:23:56
Thanks for splitting the patches: I will try to go thru one-patch-at-a-time
I can see that you are modifying configure.ac to enable various codecs at build
time. Can you please explain why you are not considering to use codec-engine run
-time API to query the codec-server information. I guess querying runtime will
give us much better flexibility than hard-coding at the build time. CE has two
a) Engine_getNumAlgs - Get the number of algorithms configured into an
b) Engine_getAlgInfo - Get details of an algorithm configured into an
By using above two API's you can very easily query the codec inside the server.
Please refer the codec-engine doc for more details.
Adddate: 2009-11-09 11:35:45
Sorry I wasnt aware of such API. Will read more and see how I can apply them in
the code. Meanwhile can you please continue review other patches.
Adddate: 2009-11-09 19:18:40
np, in case if you want to see how this can be used then check this dmai branch
FYI, in next version of dmai update we will get VdecU/IdecU/AudU/VencU....
objects - these can be used to encode/decode either XDM 0.9 or 1.x or 2.x (in
future). While refactoring gstreamer code we should also keep in mind that we
are not making real-hard to update to next dmai version or ce version and also
adding new platform should be as easier as what we have today.
DMAI is also open-source project like gst-ti and if we think we need some more
changes in dmai to help our gst-ti plugin then please submit enhancement (or
patches) in DMAI.
Adddate: 2009-11-26 06:37:41
Attached the design details documemt, that will give more detailed insight of
this patch and future ones.
Adddate: 2009-11-30 15:44:38
Thanks for the design doc, its very useful during patch review. Few questions
/* Data definition for each instance of encoder */
GstStaticCaps *srcCaps, *sinkCaps;
Looks like with this design a element will support exactly one input and one
E.g dmaienc_mpeg4 will accept only "UYVY" or some other
fourcc. And similarly dmaidec_mpeg4 will output only "UYVY" or
Please correct me if i'm wrong here ?? Please note that one some platforms
encoder support two or more input fourcc and similarly decoder can be configured
to output two or more output fourcc. e.g With current design, on DM6467 encoder
element can accept either Y8C8 or NV12 fourcc's. And similarly on DM365 MPEG4
encoder can accept either NV12 or UYVY fourcc. And not to mention, one some
platforms decoders can be configured to output YUV 4:2:0 (I420) fourcc.
2) Who is defining "PLATFORM" variable in patch 002 ?
+#if PLATFORM == dm6467
+ dmaienc->colorSpace = ColorSpace_YUV422PSEMI;
+ dmaienc->colorSpace = ColorSpace_UYVY;
I think your patch 002, 003... depends very heavily on patch 001. Please update
your 001 patch first. I think thats really key one.
Adddate: 2009-11-30 16:41:07
You said dmaienc.c is common code for all encoder stuff then why you are adding
XDM 0.9 video specific header files here. e.g
Adddate: 2009-12-01 02:53:12
1) "Looks like with this design a element will support exactly one
input and one
- the srcCaps & sinkCaps are the pad templates, so one can have as many
caps as it needs
2) Who is defining "PLATFORM" variable in patch 002 ?
its passed from configure script so yes you are right basically patch 001, will
update on it
3) "+#include <ti/xdais/dm/ivideo.h>"
Its not needed, sorry seems to be a misunderstanding.
Awaiting your further comments
Adddate: 2010-10-19 15:51:49
Patches do not follow gst-dmai patch standard.