[FFmpeg-devel] GSOC update on the CDXL demuxer

[Michael Niedermayer]

On Tue, Apr 14, 2009 at 11:39:59AM -0500, Erion Omeri wrote:
> I initially was going to work on the decoder but it was suggested to at
> least do the demuxer.
> I was stuck for a long time and I did not realize I was missing the SDL
> until Ronald Bultje started helping me, it was great help!
> I have a better understanding of the whole environment now.
> 
> I will continue working on this, although I understand it might be late to
> complete the task on time.

Yes, it seems our 10th slot has been given back to google.
Still everyone is of course very welcome to continue their qualification task,
you could see it as a free course in advanced C coding


[...]

> #undef printf 

that does not belong in there
also all trailing whitespace must be removed


[...]
> typedef struct{
>     char Type;          // byte 0
>     char Info;          // byte 1 ( bit 0,1,2, bit 3, bit 5,6,7)
>     long CurrSize;      // byte 2, 3, 4, 5
>     long PrevSize;      // byte 6, 7, 8, 9
>     int Resolution;     // byte 10, 11
>     int CurrFrameNum;   // byte 12, 13
>     int Width;          // byte 14, 15
>     int Height;         // byte 16, 17,
>     int Depth;          // byte 18, 19
>     int CMapSize;       // byte 20, 21
>     int RawSoundSize;   // byte 22, 23
>     long Reserved1;     // byte 24, 25, 26, 27,
>     long Reserved2;     // byte 28, 29, 30, 31

Struct fields arent capatilized in ffmpeg normally
also the comments should be doxygen compatible


[...]
> static int cdxl_read_header(AVFormatContext *s, AVFormatParameters *ap)
> { 
>     printf("inHEADER, ");
> 
>     ByteIOContext *pb = s->pb; 
>     AVStream *st;  
> 
>     CDXLContext *cdxl = s->priv_data;
>     cdxl_read_only_header(pb, cdxl);
>  

>     #if 0    
>     debug_header(cdxl);
>     #endif

such stuff does not belong in the final version


>      
>     /* Audio, Uncompressed */
>     st = av_new_stream(s, 0);
>     if (!st)
>         return -1; 

>     st->codec->codec_type = CODEC_TYPE_AUDIO; 
>     st->codec->codec_id = CODEC_ID_PCM_S8;
>     st->codec->sample_rate = CDXL_FRAME_RATE; 

vertical align


>      
>     /** If Info=00001000 than stereo **/

>     st->codec->channels = CDXL_MONO;

1 seems more understandable than CDXL_MONO


>     if( CDXL_STEREO_MASK == ( CDXL_STEREO_MASK & cdxl->Info ) ) 

this can be simpified


[...]
> static int cdxl_read_packet(AVFormatContext *s, AVPacket *pkt)
> {    
>      
>     printf("inpacket, ");
> 
>     int ret = 0, videoSize; 
>     CDXLContext *cdxl = s->priv_data; 
>     ByteIOContext *pb = s->pb;

>     AVStream *st  = av_new_stream(s, 0);

this does not look good


>     if (!st)
>         return AVERROR(ENOMEM); 
>      
>     /** Skip Palette **/
>     url_fskip(pb, CDXL_PALETE_SIZE);
>    

>     cdxl->VideoFrameCount += 1;  

cdxl->VideoFrameCount++


> 
>     /*Skip Video */
>     videoSize = cdxl->CurrSize - cdxl->RawSoundSize - CDXL_PALETE_SIZE - CDXL_HEADER_SIZE;
>     url_fskip(pb, videoSize);
>     
>     ret = av_get_packet(s->pb, pkt, cdxl->RawSoundSize); 
>     if(ret != cdxl->RawSoundSize)
>         return AVERROR(EIO); 
> 
>     pkt->stream_index = CDXL_AUDIO_STREAM_ID;   
> 
>     /* Audio Packet */
>     pkt->pts = cdxl->AudioFrameCount;  
>    
>     /** If stereao, the actually sound size is half **/
>     if( 2 == s->streams[CDXL_AUDIO_STREAM_ID]->codec->channels  )

>         cdxl->AudioFrameCount += ( cdxl->RawSoundSize / 2 );

superflous ()


[...]
> AVInputFormat cdxl_demuxer = {
>     "CDXL",
>     NULL_IF_CONFIG_SMALL("CDXL Video"),
>     sizeof(CDXLContext),
>     NULL, 
>     cdxl_read_header,
>     cdxl_read_packet,
>     .extensions="cdxl",
> };

a probe function is missing

[...]
-- 
Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

Its not that you shouldnt use gotos but rather that you should write
readable code and code with gotos often but not always is less readable
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 189 bytes
Desc: Digital signature
URL: <http://lists.mplayerhq.hu/pipermail/ffmpeg-devel/attachments/20090414/f79b8099/attachment.pgp>