[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