[FFmpeg-devel] [RFC/PATCH 6/8] tidsp: add mp4v configuration

[Felipe Contreras]

On Wed, Sep 8, 2010 at 8:17 AM, Reimar D?ffinger
<Reimar.Doeffinger at gmx.de> wrote:
> On Wed, Sep 08, 2010 at 02:01:26AM +0300, Felipe Contreras wrote:
>> On Tue, Sep 7, 2010 at 10:54 PM, Reimar D?ffinger
>> <Reimar.Doeffinger at gmx.de> wrote:
>> > On Tue, Sep 07, 2010 at 10:26:05PM +0300, Felipe Contreras wrote:
>> >> On Tue, Sep 7, 2010 at 10:15 PM, Reimar D?ffinger
>> >> <Reimar.Doeffinger at gmx.de> wrote:
>> >> > On Tue, Sep 07, 2010 at 12:44:12PM +0300, Felipe Contreras wrote:
>> >> >> On Tue, Sep 7, 2010 at 5:45 AM, Ronald S. Bultje <rsbultje at gmail.com> wrote:
>> >> >> > On Sun, Sep 5, 2010 at 6:15 PM, Felipe Contreras
>> >> >> > <felipe.contreras at gmail.com> wrote:
>> >> >> >> +struct mp4vdec_args {
>> >> >> >> + ? ? ? uint32_t size;
>> >> >> >> + ? ? ? uint16_t num_streams;
>> >> >> >> +
>> >> >> >> + ? ? ? uint16_t in_id;
>> >> >> >> + ? ? ? uint16_t in_type;
>> >> >> >> + ? ? ? uint16_t in_count;
>> >> >> >> +
>> >> >> >> + ? ? ? uint16_t out_id;
>> >> >> >> + ? ? ? uint16_t out_type;
>> >> >> >> + ? ? ? uint16_t out_count;
>> >> >> >> +
>> >> >> >> + ? ? ? uint16_t reserved;
>> >> >> >> +
>> >> >> >> + ? ? ? uint32_t max_width;
>> >> >> >> + ? ? ? uint32_t max_height;
>> >> >> >> + ? ? ? uint32_t color_format;
>> >> >> >> + ? ? ? uint32_t max_framerate;
>> >> >> >> + ? ? ? uint32_t max_bitrate;
>> >> >> >> + ? ? ? uint32_t endianness;
>> >> >> >> + ? ? ? uint32_t profile;
>> >> >> >> + ? ? ? int32_t max_level;
>> >> >> >> + ? ? ? uint32_t mode;
>> >> >> >> + ? ? ? int32_t preroll;
>> >> >> >> + ? ? ? uint32_t display_width;
>> >> >> >> +};
>> >> >> >
>> >> >> > What are all these?
>> >> >>
>> >> >> These are arguments needed to start the DSP algorithm.
>> >> >
>> >> > Assuming you mean they are directly pass to hardware:
>> >> > I don't think a struct is a good way to do this,
>> >> > you're relying on the compiler not inserting some
>> >> > padding just because it likes to (e.g. on a machine
>> >> > that can only do 32 bit read and writes it might allocate
>> >> > 4 bytes for all the uint16_t - I admit I haven't double-
>> >> > checked this is indeed allowed).
>> >>
>> >> You mean padding at the end, or in the middle? Appending stuff at the
>> >> end is generally not a problem (although some algos check the size
>> >> specified). If it's in the middle... how do you suggest fixing that?
>> >
>> > Building the memory layout manually using bytestream_put_...(),
>> > using comments to describe what's written where (since there's
>> > no more variable names to guess it from anymore).
>>
>> But the code in the 'tidsp' apparently wouldn't belong to FFmpeg, so
>> the bytestream_* utilities are not available.
>
> It's not any complex code, since it doesn't have to be efficient it can
> be reimplemented in a few minutes...

Well sure, now that I've done all the work of dissecting TI's openmax,
lcml and libdspbridge it is relatively simple. But if your eyes have a
high tolerance to bleeding, I invite you to see some of the original
code [1][2][3][4][5] and judge how easy it would be to reimplement
that.

And the code is *extremely* efficient, the result of years of
profiling the code both in user-space and kernel-space, discussions,
reorganization, etc. I mean the code inside the 'tidsp' directory,
what is outside, which is specific to FFmpeg, still needs a lot of
optimization work, but it's new, so that's expected.

[1] http://lxr.post-tech.com/source/hardware/ti/omap3/omx/video/src/openmax_il/video_decode/inc/OMX_VideoDec_Utils.h
[2] http://lxr.post-tech.com/source/hardware/ti/omap3/omx/video/src/openmax_il/video_decode/src/OMX_VideoDec_Utils.c
[3] http://lxr.post-tech.com/source/hardware/ti/omap3/omx/video/src/openmax_il/video_decode/src/OMX_VideoDec_Thread.c
[4] http://lxr.post-tech.com/source/hardware/ti/omap3/omx/video/src/openmax_il/video_decode/src/OMX_VideoDecoder.c
[5] http://lxr.post-tech.com/source/hardware/ti/omap3/omx/system/src/openmax_il/lcml/src/LCML_DspCodec.c

-- 
Felipe Contreras