[Ffmpeg-devel] [PATCH] from DivX, Part 1: cosmectic changes

Benjamin Larsson banan
Fri Dec 16 10:04:10 CET 2005


Hi,

Steve Lhomme wrote:

> Hi everyone,
>
> This is the first patch from DivX in order to make our code as similar
> as the official FFMPEG as possible.
>
> This patch includes only the cosmetic changes so that after that we
> can proceed with the real juice (otherwise the noise from these
> changes is too big to find the actual fixes).
>
> It has :
> - tab removal (yes the official code has tabs)
> - use av_log instead of fprintf
> - missing includes (help MSVC at least and won't hurt anyone)
> - a few #ifdef to hide code when it's not enabled in the main config
> (to avoid having to fix MSVC portability issues)
> - more PRId64, PRIx64, etc
>
- asserts added

> Have fun.

Most of it looks ok but I think the patch needs one pure cosmetical part
and another functional one. And
the cosmetical part of the patch needs some work. I looked through the
most obvious parts.

>@@ -820,10 +820,10 @@
>                     padcolor);
>         }
>         
>-	if (enc->pix_fmt != PIX_FMT_YUV420P) {
>+        if (enc->pix_fmt != PIX_FMT_YUV420P) {
>             int size;
>-	    
>-	    av_free(buf);
>+            
>  
>
Tabs -> whitespace.

>+            av_free(buf);
>             /* create temporary picture */
>             size = avpicture_get_size(enc->pix_fmt, enc->width, enc->height);
>             buf = av_malloc(size);
>@@ -1911,16 +1911,16 @@
>         int out_file_index = meta_data_maps[i].out_file;
>         int in_file_index = meta_data_maps[i].in_file;
>         if ( out_file_index < 0 || out_file_index >= nb_output_files ) {
>-            fprintf(stderr, "Invalid output file index %d map_meta_data(%d,%d)\n", out_file_index, out_file_index, in_file_index);
>+            av_log(NULL, AV_LOG_ERROR, "Invalid output file index %d map_meta_data(%d,%d)\n", out_file_index, out_file_index, in_file_index);
>             ret = -EINVAL;
>             goto fail;
>         }
>         if ( in_file_index < 0 || in_file_index >= nb_input_files ) {
>-            fprintf(stderr, "Invalid input file index %d map_meta_data(%d,%d)\n", in_file_index, out_file_index, in_file_index);
>+            av_log(NULL, AV_LOG_ERROR, "Invalid input file index %d map_meta_data(%d,%d)\n", in_file_index, out_file_index, in_file_index);
>             ret = -EINVAL;
>             goto fail;
>-        }		
>-		 
>+        }                
>+                 
>  
>
Tabs -> whitespace.

>         out_file = output_files[out_file_index];
>         in_file = input_files[in_file_index];
> 
>@@ -1933,12 +1933,12 @@
>         out_file->track = in_file->track;
>         strcpy(out_file->genre, in_file->genre);
>     }
>-	
>+        
>  
>
Tabs -> whitespace.

>     /* open files and write file headers */
>     for(i=0;i<nb_output_files;i++) {
>         os = output_files[i];
>         if (av_write_header(os) < 0) {
>-            fprintf(stderr, "Could not write header for output file #%d (incorrect codec parameters ?)\n", i);
>+            av_log(NULL, AV_LOG_ERROR, "Could not write header for output file #%d (incorrect codec parameters ?)\n", i);
>             ret = -EINVAL;
>             goto fail;
>         }
>@@ -3438,13 +3438,13 @@
>         }
> 
>         if (!oc->nb_streams) {
>-            fprintf(stderr, "No audio or video streams available\n");
>+            av_log(NULL, AV_LOG_ERROR, "No audio or video streams available\n");
>             exit(1);
>-        }
>+        }		
>  
>
Tabs added.

> 
>         oc->timestamp = rec_timestamp;
>-	    
>-	if (str_title)
>+            
>  
>
Tabs -> whitespace.

>+        if (str_title)
>             pstrcpy(oc->title, sizeof(oc->title), str_title);
>         if (str_author)
>             pstrcpy(oc->author, sizeof(oc->author), str_author);
>@@ -251,9 +249,9 @@
>                 sub_header->num_rects = 1;
>                 sub_header->rects[0].rgba_palette = av_malloc(4 * 4);
>                 decode_rle(bitmap, w * 2, w, h / 2,
>-                           buf, offset1 * 2, buf_size);
>+                           buf, offset1 * 2, buf_size);				
>  
>
Tabs added.

>                 decode_rle(bitmap + w, w * 2, w, h / 2,
>-                           buf, offset2 * 2, buf_size);
>+                           buf, offset2 * 2, buf_size);			    
>  
>
Tabs added.

>                 guess_palette(sub_header->rects[0].rgba_palette,
>                               palette, alpha, 0xffff00);
>                 sub_header->rects[0].x = x1;
>@@ -410,7 +408,7 @@
>     "dvdsub",
>     CODEC_TYPE_SUBTITLE,
>     CODEC_ID_DVD_SUBTITLE,
>-    sizeof(DVDSubContext),
>+    0,
>  
>
?

>     dvdsub_init_decoder,
>     NULL,
>     dvdsub_close_decoder,
>Index: libavcodec/faac.c
>===================================================================
>RCS file: /cvsroot/ffmpeg/ffmpeg/libavcodec/faac.c,v
>retrieving revision 1.3
>diff -u -r1.3 faac.c
>--- libavcodec/faac.c	21 Aug 2005 20:27:00 -0000	1.3
>+++ libavcodec/faac.c	15 Dec 2005 01:59:45 -0000
>Index: libavcodec/loco.c
>===================================================================
>RCS file: /cvsroot/ffmpeg/ffmpeg/libavcodec/loco.c,v
>retrieving revision 1.5
>diff -u -r1.5 loco.c
>--- libavcodec/loco.c	8 Apr 2005 21:34:48 -0000	1.5
>+++ libavcodec/loco.c	15 Dec 2005 02:06:59 -0000
>@@ -241,14 +241,14 @@
>         l->lossy = 0;
>         break;
>     case 2:
>-        l->lossy = LE_32(avctx->extradata + 8);
>+        l->lossy = LE_32((char*)avctx->extradata + 8);
>  
>
Not int8_t ?

>         break;
>     default:
>-        l->lossy = LE_32(avctx->extradata + 8);
>+        l->lossy = LE_32((char*)avctx->extradata + 8);
>  
>
Not int8_t ?

>         av_log(avctx, AV_LOG_INFO, "This is LOCO codec version %i, please upload file for study\n", version);
>     }
>     
>-    l->mode = LE_32(avctx->extradata + 4);
>+    l->mode = LE_32((char*)avctx->extradata + 4);
>  
>
Not int8_t ?

>     switch(l->mode) {
>     case LOCO_CYUY2: case LOCO_YUY2: case LOCO_UYVY:
>         avctx->pix_fmt = PIX_FMT_YUV422P;
>Index: libavcodec/mem.c
>===================================================================
>RCS file: /cvsroot/ffmpeg/ffmpeg/libavcodec/mem.c,v
>retrieving revision 1.12
>diff -u -r1.12 mem.c
>--- libavcodec/mem.c	24 Feb 2005 19:08:49 -0000	1.12
>+++ libavcodec/mem.c	15 Dec 2005 02:08:08 -0000
>@@ -44,7 +44,7 @@
>  */
> void *av_malloc(unsigned int size)
> {
>-    void *ptr;
>+    char *ptr;
>  
>
Not int8_t ?

> #ifdef MEMALIGN_HACK
>     int diff;
> #endif
>@@ -57,7 +57,7 @@
>     ptr = malloc(size+16+1);
>     diff= ((-(int)ptr - 1)&15) + 1;
>     ptr += diff;
>-    ((char*)ptr)[-1]= diff;
>+    ptr[-1]= diff;
> #elif defined (HAVE_MEMALIGN) 
>     ptr = memalign(16,size);
>     /* Why 64? 
>         
>Index: libavcodec/ws-snd1.c
>===================================================================
>RCS file: /cvsroot/ffmpeg/ffmpeg/libavcodec/ws-snd1.c,v
>retrieving revision 1.1
>diff -u -r1.1 ws-snd1.c
>--- libavcodec/ws-snd1.c	28 Mar 2005 18:05:25 -0000	1.1
>+++ libavcodec/ws-snd1.c	15 Dec 2005 02:16:29 -0000
>@@ -137,7 +137,7 @@
>     "ws_snd1",
>     CODEC_TYPE_AUDIO,
>     CODEC_ID_WESTWOOD_SND1,
>-    sizeof(WSSNDContext),
>+    0,
>  
>
?

>     ws_snd_decode_init,
>     NULL,
>     NULL,
>Index: libavformat/asf.c
>===================================================================
>RCS file: /cvsroot/ffmpeg/ffmpeg/libavformat/asf.c,v
>retrieving revision 1.87
>diff -u -r1.87 asf.c
>--- libavformat/asf.c	12 Dec 2005 01:56:46 -0000	1.87
>+++ libavformat/asf.c	15 Dec 2005 01:19:05 -0000
>@@ -441,7 +441,7 @@
>         rsize+=2;
> /*    }else{
>         if (!url_feof(pb))
>-	    printf("ff asf bad header %x  at:%lld\n", c, url_ftell(pb));
>+	    printf("ff asf bad header %x  at:%"PRId64"\n", c, url_ftell(pb));
>  
>
No av_log conversion?

> 	return AVERROR_IO;*/
>     }
> 
> 
>Index: libavformat/aviobuf.c
>===================================================================
>RCS file: /cvsroot/ffmpeg/ffmpeg/libavformat/aviobuf.c,v
>retrieving revision 1.31
>diff -u -r1.31 aviobuf.c
>--- libavformat/aviobuf.c	23 Sep 2005 00:25:41 -0000	1.31
>+++ libavformat/aviobuf.c	15 Dec 2005 01:22:58 -0000
>@@ -713,8 +713,8 @@
>     else
>         io_buffer_size = 1024;
>         
>-    if(sizeof(DynBuffer) + io_buffer_size < io_buffer_size)
>-        return -1;
>+    assert(sizeof(DynBuffer) + io_buffer_size >= io_buffer_size);
>+
>  
>
?

>     d = av_malloc(sizeof(DynBuffer) + io_buffer_size);
>     if (!d)
>         return -1;
>Index: libavformat/mov.c
>===================================================================
>RCS file: /cvsroot/ffmpeg/ffmpeg/libavformat/mov.c,v
>retrieving revision 1.94
>diff -u -r1.94 mov.c
>--- libavformat/mov.c	12 Dec 2005 01:56:46 -0000	1.94
>+++ libavformat/mov.c	4 Nov 2005 09:56:18 -0000
>@@ -17,6 +17,7 @@
>  * Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA  02111-1307  USA
>  */
> 
>+#include <unistd.h>
> #include <limits.h>
>  
> #include "avformat.h"
>@@ -77,7 +78,7 @@
> /* some streams in QT (and in MP4 mostly) aren't either video nor audio */
> /* so we first list them as this, then clean up the list of streams we give back, */
> /* getting rid of these */
>-#define CODEC_TYPE_MOV_OTHER	(enum CodecType) 2
>+#define CODEC_TYPE_MOV_OTHER        (enum CodecType) 2
> 
> static const CodecTag mov_video_tags[] = {
> /*  { CODEC_ID_, MKTAG('c', 'v', 'i', 'd') }, *//* Cinepak */
>@@ -179,25 +180,25 @@
> 
>     /* 0x03 ESDescrTag */
>     uint16_t es_id;
>-#define MP4ODescrTag			0x01
>-#define MP4IODescrTag			0x02
>-#define MP4ESDescrTag			0x03
>-#define MP4DecConfigDescrTag		0x04
>-#define MP4DecSpecificDescrTag		0x05
>-#define MP4SLConfigDescrTag		0x06
>-#define MP4ContentIdDescrTag		0x07
>-#define MP4SupplContentIdDescrTag	0x08
>-#define MP4IPIPtrDescrTag		0x09
>-#define MP4IPMPPtrDescrTag		0x0A
>-#define MP4IPMPDescrTag			0x0B
>-#define MP4RegistrationDescrTag		0x0D
>-#define MP4ESIDIncDescrTag		0x0E
>-#define MP4ESIDRefDescrTag		0x0F
>-#define MP4FileIODescrTag		0x10
>-#define MP4FileODescrTag		0x11
>-#define MP4ExtProfileLevelDescrTag	0x13
>-#define MP4ExtDescrTagsStart		0x80
>-#define MP4ExtDescrTagsEnd		0xFE
>+#define MP4ODescrTag                        0x01
>+#define MP4IODescrTag                        0x02
>+#define MP4ESDescrTag                        0x03
>+#define MP4DecConfigDescrTag                0x04
>+#define MP4DecSpecificDescrTag                0x05
>+#define MP4SLConfigDescrTag                0x06
>+#define MP4ContentIdDescrTag                0x07
>+#define MP4SupplContentIdDescrTag        0x08
>+#define MP4IPIPtrDescrTag                0x09
>+#define MP4IPMPPtrDescrTag                0x0A
>+#define MP4IPMPDescrTag                        0x0B
>+#define MP4RegistrationDescrTag                0x0D
>+#define MP4ESIDIncDescrTag                0x0E
>+#define MP4ESIDRefDescrTag                0x0F
>+#define MP4FileIODescrTag                0x10
>+#define MP4FileODescrTag                0x11
>+#define MP4ExtProfileLevelDescrTag        0x13
>+#define MP4ExtDescrTagsStart                0x80
>+#define MP4ExtDescrTagsEnd                0xFE
>     uint8_t  stream_priority;
> 
>  
>
Well the tabs are gone but it doesn't look good.

>     /* 0x04 DecConfigDescrTag */
>@@ -242,8 +243,8 @@
>     long sample_to_chunk_sz;
>     MOV_sample_to_chunk_tbl *sample_to_chunk;
>     long sample_to_chunk_index;
>-    int sample_to_time_index;	 
>-    long sample_to_time_sample;	 
>+    int sample_to_time_index;         
>+    long sample_to_time_sample;         
>  
>
Tabs -> whitespace.

>     uint64_t sample_to_time_time;    
>     int sample_to_ctime_index;
>     int sample_to_ctime_sample;
>@@ -2001,18 +2002,18 @@
> 
>     mov->next_chunk_offset = offset + size;
>     
>-    /* find the corresponding dts */	 
>-    if (sc && sc->sample_to_time_index < sc->stts_count && pkt) {	 
>+    /* find the corresponding dts */         
>  
>
Tabs -> whitespace.

>+    if (sc && sc->sample_to_time_index < sc->stts_count && pkt) {         
>       unsigned int count;
>       uint64_t dts, pts;
>       unsigned int duration = sc->stts_data[sc->sample_to_time_index].duration;
>       count = sc->stts_data[sc->sample_to_time_index].count;
>-      if ((sc->sample_to_time_sample + count) < sc->current_sample) {	 
>-        sc->sample_to_time_sample += count;	 
>-        sc->sample_to_time_time   += count*duration;	 
>-        sc->sample_to_time_index ++;	 
>+      if ((sc->sample_to_time_sample + count) < sc->current_sample) {         
>+        sc->sample_to_time_sample += count;         
>+        sc->sample_to_time_time   += count*duration;         
>+        sc->sample_to_time_index ++;         
>         duration = sc->stts_data[sc->sample_to_time_index].duration;
>-      }	 
>+      }         
>  
>
Tabs -> whitespace.

>       dts = sc->sample_to_time_time + (sc->current_sample-1 - sc->sample_to_time_sample) * (int64_t)duration;
>         /* find the corresponding pts */
>         if (sc->sample_to_ctime_index < sc->ctts_count) {
>@@ -2030,7 +2031,7 @@
>         pkt->pts = pts;
>         pkt->dts = dts;
> #ifdef DEBUG
>-    av_log(NULL, AV_LOG_DEBUG, "stream #%d smp #%ld dts = %lld pts = %lld (smp:%ld time:%lld idx:%d ent:%d count:%d dur:%d)\n"
>+    av_log(NULL, AV_LOG_DEBUG, "stream #%d smp #%ld dts = %"PRId64" pts = %"PRId64" (smp:%ld time:%"PRId64" idx:%d ent:%d count:%d dur:%d)\n"
>       , pkt->stream_index, sc->current_sample-1, pkt->dts, pkt->pts
>       , sc->sample_to_time_sample
>       , sc->sample_to_time_time
>@@ -2063,8 +2064,8 @@
>     int64_t sample_file_offset;
>     int32_t first_chunk_sample;
>     int32_t sample_to_chunk_idx;
>-    int sample_to_time_index;	 
>-    long sample_to_time_sample = 0;	 
>+    int sample_to_time_index;         
>+    long sample_to_time_sample = 0;         
>     uint64_t sample_to_time_time = 0;      
>  
>
Tabs -> whitespace.

>     int mov_idx;
> 
>@@ -2087,7 +2088,7 @@
> 
>     // Step 2. Find the corresponding sample using the Time-to-sample atom (stts) */
> #ifdef DEBUG
>-  av_log(s, AV_LOG_DEBUG, "Searching for time %li in stream #%i (time_scale=%i)\n", (long)timestamp, mov_idx, sc->time_scale);
>+  av_log(s, AV_LOG_DEBUG, "Searching for time %li in stream #%i (time_scale=%i)\n", (long)sample_time, mov_idx, sc->time_scale);
> #endif
>     start_time = 0; // FIXME use elst atom
>     sample = 1; // sample are 0 based in table
>@@ -2100,8 +2101,8 @@
> //av_log(s, AV_LOG_DEBUG, "> sample_time %lli \n", (long)sample_time);                
> //av_log(s, AV_LOG_DEBUG, "> count=%i duration=%i\n", count, duration);        
>         if ((start_time + count*duration) > sample_time) {
>-            sample_to_time_time = start_time;	 
>-            sample_to_time_index = i;	 
>+            sample_to_time_time = start_time;         
>+            sample_to_time_index = i;         
>             sample_to_time_sample = sample;         
>  
>
Tabs -> whitespace.

>             sample += (sample_time - start_time) / duration;
>             break;
>@@ -2109,7 +2110,7 @@
>         sample += count;
>         start_time += count * duration;
>     }   
>-    sample_to_time_time = start_time;	 
>+    sample_to_time_time = start_time;         
>  
>
Tabs -> whitespace.

>     sample_to_time_index = i;       
>     /* NOTE: despite what qt doc say, the dt value (Display Time in qt vocabulary) computed with the stts atom
>        is a decoding time stamp (dts) not a presentation time stamp. And as usual dts != pts for stream with b frames */
>@@ -2234,19 +2235,19 @@
>         }
>         msc->current_sample += (msc->next_chunk - (msc->sample_to_chunk[msc->sample_to_chunk_index].first - 1)) * sc->sample_to_chunk[msc->sample_to_chunk_index].count;
>         msc->left_in_chunk = msc->sample_to_chunk[msc->sample_to_chunk_index].count - 1;
>-        // Find corresponding position in stts (used later to compute dts)	 
>-        sample = 0;	 
>-        start_time = 0;	 
>-        for (msc->sample_to_time_index = 0; msc->sample_to_time_index < msc->stts_count; msc->sample_to_time_index++) {	 
>+        // Find corresponding position in stts (used later to compute dts)         
>+        sample = 0;         
>+        start_time = 0;         
>+        for (msc->sample_to_time_index = 0; msc->sample_to_time_index < msc->stts_count; msc->sample_to_time_index++) {         
>  
>
Tabs -> whitespace.

>             count = msc->stts_data[msc->sample_to_time_index].count;
>             duration = msc->stts_data[msc->sample_to_time_index].duration;
>-            if ((sample + count - 1) > msc->current_sample) {	 
>-                msc->sample_to_time_time = start_time;	 
>-                msc->sample_to_time_sample = sample;	 
>-                break;	 
>-            }	 
>-            sample += count;	 
>-            start_time += count * duration;	 
>+            if ((sample + count - 1) > msc->current_sample) {         
>+                msc->sample_to_time_time = start_time;         
>+                msc->sample_to_time_sample = sample;         
>+                break;         
>+            }         
>+            sample += count;         
>+            start_time += count * duration;         
>  
>
Tabs -> whitespace.

>         }
>         sample = 0;
>         for (msc->sample_to_ctime_index = 0; msc->sample_to_ctime_index < msc->ctts_count; msc->sample_to_ctime_index++) {
>Index: libavformat/utils.c
>===================================================================
>RCS file: /cvsroot/ffmpeg/ffmpeg/libavformat/utils.c,v
>retrieving revision 1.169
>diff -u -r1.169 utils.c
>--- libavformat/utils.c	12 Dec 2005 01:56:46 -0000	1.169
>+++ libavformat/utils.c	15 Dec 2005 01:34:18 -0000
>@@ -192,12 +192,11 @@
> int av_new_packet(AVPacket *pkt, int size)
> {
>     void *data;
>-    if((unsigned)size > (unsigned)size + FF_INPUT_BUFFER_PADDING_SIZE)
>-        return AVERROR_NOMEM;        
>+    assert((unsigned)size <= (unsigned)size + FF_INPUT_BUFFER_PADDING_SIZE);
>  
>
?

>     data = av_malloc(size + FF_INPUT_BUFFER_PADDING_SIZE);
>     if (!data)
>         return AVERROR_NOMEM;
>-    memset(data + size, 0, FF_INPUT_BUFFER_PADDING_SIZE);
>+    memset((char*)data + size, 0, FF_INPUT_BUFFER_PADDING_SIZE);
>  
>
uint8_t?

> 
>     av_init_packet(pkt);
>     pkt->data = data; 
>@@ -239,8 +238,7 @@
>         uint8_t *data;
>         /* we duplicate the packet and don't forget to put the padding
>            again */
>-        if((unsigned)pkt->size > (unsigned)pkt->size + FF_INPUT_BUFFER_PADDING_SIZE)
>-            return AVERROR_NOMEM;        
>+		assert((unsigned)pkt->size <= (unsigned)pkt->size + FF_INPUT_BUFFER_PADDING_SIZE);
>  
>
?

>         data = av_malloc(pkt->size + FF_INPUT_BUFFER_PADDING_SIZE);
>         if (!data) {
>             return AVERROR_NOMEM;
>  
>
MvH
Benjamin Larsson

-- 
"incorrect information" is an oxymoron. Information is, by definition, factual, correct.





More information about the ffmpeg-devel mailing list