[Ffmpeg-devel] [PATCH proposal] ffmpeg.c - pre_process_video_frame()

Alexey Sidelnikov avs99
Thu Oct 26 03:33:39 CEST 2006


Hello!

As far as I understand, if you want to "modify" somehow the picture 
after it was decoded but before it is fed to the encoder, you should use 
a temporary buffer, is that correct? The reason is (it's my guess only) 
is that  AVFrame picture from output_packet() function points to the 
buffer somewhere inside decoder, and you are not allowed to change it. 
Is that also correct?

If so, then I'd say that current implementation of 
pre_process_video_frame() function is wrong. If both deinterlace and 
vhook are used, and deinterlacing failed (basically that means picture 
is not interlaced, and nothing bad is here I'd say), then vhook will use 
original picture and will corrupt the video file. Also it is hard to 
figure out how to add extra preprocessing. I changed 
pre_process_video_frame() function to address both issues.

Another conclusion I got is that pre_process_video_frame() was designed 
to modify frames which will be written to the movie, right? If so, then 
I think it should be called only before actual encoding the frame, i.e. 
right before do_video_out() call. I changed output_packet() function 
accordingly.

Also in this patch I added checks (fixed, thanks Michael) for direct 
video stream copying from here:

news://news.gmane.com:119/20061025102127.GF5556 at MichaelsNB


I also have another improvement which can affect the performance I think 
- why not to make the buffer for preprocessing static and malloc/free it 
only once? Now it is malloced/freed every video frame. However, that can 
be done relatively simple only: we have no more than one video stream 
(well, not a big deal to extend to several streams actually) AND video 
stream picture size is constant during the time. Does that sound 
reasonable? Shall I make the patch?

Ohh, and one more thing - I added AVOutputStream parameter to the 
pre_process_video_frame(). I think it can be useful somehow (for 
example, getting pts or whatever), and it comes for free... so why not?


Thanks,

Alexey




Index: ffmpeg.c
===================================================================
--- ffmpeg.c	(revision 6791)
+++ ffmpeg.c	(working copy)
@@ -588,48 +588,72 @@
      }
  }

-static void pre_process_video_frame(AVInputStream *ist, AVPicture 
*picture, void **bufp)
+static void pre_process_video_frame(AVInputStream *ist, AVOutputStream 
*ost, AVPicture *pic_src, void **bufp)
  {
      AVCodecContext *dec;
-    AVPicture *picture2;
-    AVPicture picture_tmp;
+    AVPicture pic_tmp;
      uint8_t *buf = 0;
+    int size;
+    int preprocess_successed = 0;
+	
+    if (!do_deinterlace && !using_vhook)
+        return;

+    if (!ist->decoding_needed) {
+        fprintf(stderr, "video frame can not be preprocessed if video 
stream is copied\n");
+        exit(-1);
+    }
+
      dec = ist->st->codec;

+    /* create temporary picture */
+    size = avpicture_get_size(dec->pix_fmt, dec->width, dec->height);
+    buf = av_malloc(size);
+    if (!buf) {
+        av_log(NULL, AV_LOG_ERROR, "Can't allocate buffer for video 
preprocessing\n");
+        exit(-1);
+    }
+
+    avpicture_fill(&pic_tmp, buf, dec->pix_fmt, dec->width, dec->height);
+
      /* deinterlace : must be done before any resize */
-    if (do_deinterlace || using_vhook) {
-        int size;
+    if (do_deinterlace) {
+        if (avpicture_deinterlace(&pic_tmp, pic_src, dec->pix_fmt, 
dec->width, dec->height) >= 0) {
+            preprocess_successed = 1;
+        }
+    }

-        /* create temporary picture */
-        size = avpicture_get_size(dec->pix_fmt, dec->width, dec->height);
-        buf = av_malloc(size);
-        if (!buf)
-            return;
+    if (using_vhook) {
+        if (preprocess_successed == 0) {
+            img_copy(&pic_tmp, pic_src, dec->pix_fmt, dec->width, 
dec->height);
+        }
+        frame_hook_process(&pic_tmp, dec->pix_fmt, dec->width, 
dec->height);
+        preprocess_successed = 1;
+    }

-        picture2 = &picture_tmp;
-        avpicture_fill(picture2, buf, dec->pix_fmt, dec->width, 
dec->height);
+    /*  sample code for adding additional preprocess commands */
+    /*
+    if (use_my_preprocess_action) {
+        // no preprocessing was done before, so copy the sources 
picture to modify it
+        if (preprocess_successed == 0) {
+            img_copy(&pic_tmp, pic_src, dec->pix_fmt, dec->width, 
dec->height);
+        }

-        if (do_deinterlace){
-            if(avpicture_deinterlace(picture2, picture,
-                                     dec->pix_fmt, dec->width, 
dec->height) < 0) {
-                /* if error, do not deinterlace */
-                av_free(buf);
-                buf = NULL;
-                picture2 = picture;
-            }
-        } else {
-            img_copy(picture2, picture, dec->pix_fmt, dec->width, 
dec->height);
+        // if my preprocessing was successful, say it
+        if (my_preprocess_action(&pic_tmp, ...)){
+            preprocess_successed = 1;
          }
-    } else {
-        picture2 = picture;
      }
+    */

-    frame_hook_process(picture2, dec->pix_fmt, dec->width, dec->height);
-
-    if (picture != picture2)
-        *picture = *picture2;
-    *bufp = buf;
+    /* for some reasons all preprocessing actions failed */
+    if (preprocess_successed == 0) {
+        av_free(buf);
+    }
+    else {
+        *pic_src = pic_tmp;
+        *bufp = buf;
+    }
  }

  /* we begin to correct av delay at this threshold */
@@ -1040,8 +1064,7 @@
      int len, ret, i;
      uint8_t *data_buf;
      int data_size, got_picture;
-    AVFrame picture;
-    void *buffer_to_free;
+    AVFrame picture;
      static unsigned int samples_size= 0;
      static short *samples= NULL;
      AVSubtitle subtitle, *subtitle_to_free;
@@ -1148,12 +1171,6 @@
                  len = 0;
              }

-            buffer_to_free = NULL;
-            if (ist->st->codec->codec_type == CODEC_TYPE_VIDEO) {
-                pre_process_video_frame(ist, (AVPicture *)&picture,
-                                        &buffer_to_free);
-            }
-
              // preprocess audio (volume)
              if (ist->st->codec->codec_type == CODEC_TYPE_AUDIO) {
                  if (audio_volume != 256) {
@@ -1216,11 +1233,18 @@
                                  do_audio_out(os, ost, ist, data_buf, 
data_size);
                                  break;
                              case CODEC_TYPE_VIDEO:
-                                    do_video_out(os, ost, ist, 
&picture, &frame_size);
-                                    video_size += frame_size;
-                                    if (do_vstats && frame_size)
-                                        do_video_stats(os, ost, 
frame_size);
+                            {
+                                void *buffer_to_free = NULL;
+
+                                pre_process_video_frame(ist, ost, 
(AVPicture *)&picture, &buffer_to_free);
+                                do_video_out(os, ost, ist, &picture, 
&frame_size);
+                                av_free(buffer_to_free);
+
+                                video_size += frame_size;
+                                if (do_vstats && frame_size)
+                                    do_video_stats(os, ost, frame_size);
                                  break;
+                            }
                              case CODEC_TYPE_SUBTITLE:
                                  do_subtitle_out(os, ost, ist, &subtitle,
                                                  pkt->pts);
@@ -1274,7 +1298,6 @@
                          }
                      }
                  }
-            av_free(buffer_to_free);
              /* XXX: allocate the subtitles in the codec ? */
              if (subtitle_to_free) {
                  if (subtitle_to_free->rects != NULL) {
@@ -1427,6 +1450,12 @@
          os = output_files[i];
          nb_ostreams += os->nb_streams;
      }
+
+    if (nb_ostreams == 0) {
+        fprintf(stderr, "No audio, video or subtitles streams 
available\n");
+        exit(1);
+    }
+
      if (nb_stream_maps > 0 && nb_stream_maps != nb_ostreams) {
          fprintf(stderr, "Number of stream maps must match number of 
output streams\n");
          exit(1);
@@ -1544,6 +1573,11 @@
                  codec->block_align= icodec->block_align;
                  break;
              case CODEC_TYPE_VIDEO:
+                if (do_deinterlace || using_vhook) {
+                    fprintf(stderr, "Neither deinterlace, nor vhook are 
allowed during video stream copying!\nDecoding/encoding is required.\n");
+                    exit(1);
+                }
+
                  codec->pix_fmt = icodec->pix_fmt;
                  codec->width = icodec->width;
                  codec->height = icodec->height;
-------------- next part --------------
An embedded and charset-unspecified text was scrubbed...
Name: ffmpeg_preprocess.patch
URL: <http://lists.mplayerhq.hu/pipermail/ffmpeg-devel/attachments/20061026/77635047/attachment.asc>



More information about the ffmpeg-devel mailing list