[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