| Message ID | 20200405233824.29682-1-cus@passwd.hu |
|---|---|
| State | New |
| Headers | show |
| Series | [FFmpeg-devel,1/3] avdevice/xv: change codec to wrapped avframe | expand |
| Context | Check | Description |
|---|---|---|
| andriy/ffmpeg-patchwork | success | Make fate finished |
Marton Balint (12020-04-06): > Signed-off-by: Marton Balint <cus@passwd.hu> > --- > libavdevice/xv.c | 15 +++++---------- > 1 file changed, 5 insertions(+), 10 deletions(-) Thanks for working on this. Adding support for the more efficient API is a good idea. On the other hand, if applications use this device or a similar one specifically (I do in some of mine, I am certainly not alone), they would hardcode rawvideo. Therefore, we cannot remove support for rawvideo. Regards,
Am Mo., 6. Apr. 2020 um 14:31 Uhr schrieb Nicolas George <george@nsup.org>: > > Marton Balint (12020-04-06): > > Signed-off-by: Marton Balint <cus@passwd.hu> > > --- > > libavdevice/xv.c | 15 +++++---------- > > 1 file changed, 5 insertions(+), 10 deletions(-) > > Thanks for working on this. > > Adding support for the more efficient API is a good idea. > > On the other hand, if applications use this device or a similar one > specifically (I do in some of mine, I am certainly not alone), they > would hardcode rawvideo. Therefore, we cannot remove support for > rawvideo. We have done that in the past (for output formats that are not testing-only as xv). A minor bump would be nice though. Carl Eugen
Carl Eugen Hoyos (12020-04-06): > We have done that in the past (for output formats that are not > testing-only as xv). Xv is not testing only. OpenGL even less. This would be breaking actual features. What formats are you thinking of? Even if we did, it is entirely possible we were wrong then. Regards,
Am Mo., 6. Apr. 2020 um 16:39 Uhr schrieb Nicolas George <george@nsup.org>: > > Carl Eugen Hoyos (12020-04-06): > > We have done that in the past (for output formats that are not > > testing-only as xv). > > Xv is not testing only. OpenGL even less. This would be > breaking actual features. Is it possible to support both codecs? > What formats are you thinking of? Even if we did, it is > entirely possible we were wrong then. Commit 64ceeac2 changed the only supported format for the null muxer without any version bump. (While this is a very important muxer for me, I agree that my argumentation above is not completely compelling.) Carl Eugen
Carl Eugen Hoyos (12020-04-06): > Is it possible to support both codecs? Of course it it possible. And this is exactly the right thing to do (modulo the question of which raw frame API we keep). > Commit 64ceeac2 changed the only supported format for the > null muxer without any version bump. > (While this is a very important muxer for me, I agree that my > argumentation above is not completely compelling.) null is a different case: nobody will use null specifically, if they use it, they use it as a placeholder for generic code capable of handling any muxer. And since it is generic code, it will handle any codec. But anyway, you are reading this commit wrongly. It does not remove support for rawvideo in null, rawvideo is still supported in null, as any other codec. This commit only changes the default codec. Regards,
On Mon, 6 Apr 2020, Nicolas George wrote: > Marton Balint (12020-04-06): >> Signed-off-by: Marton Balint <cus@passwd.hu> >> --- >> libavdevice/xv.c | 15 +++++---------- >> 1 file changed, 5 insertions(+), 10 deletions(-) > > Thanks for working on this. > > Adding support for the more efficient API is a good idea. > > On the other hand, if applications use this device or a similar one > specifically (I do in some of mine, I am certainly not alone), they > would hardcode rawvideo. Therefore, we cannot remove support for > rawvideo. avdevice/xv and avdevice/opengl are rarely used outside of ffmpeg CLI invocations, so I don't think it is worth supporting the inferior rawvideo. If you feel strongly about this then let's drop rawvideo support after the API bump. Regards, Marton
Marton Balint (12020-04-06): > avdevice/xv and avdevice/opengl are rarely used outside of ffmpeg CLI > invocations, so I don't think it is worth supporting the inferior rawvideo. They are used. > If you feel strongly about this then let's drop rawvideo support after the > API bump. No, let's keep it. Regards,
Am Mo., 6. Apr. 2020 um 19:58 Uhr schrieb Nicolas George <george@nsup.org>: > > Carl Eugen Hoyos (12020-04-06): > > Is it possible to support both codecs? > > Of course it it possible. And this is exactly the right thing to do > (modulo the question of which raw frame API we keep). > > > Commit 64ceeac2 changed the only supported format for the > > null muxer without any version bump. > > (While this is a very important muxer for me, I agree that my > > argumentation above is not completely compelling.) > > null is a different case: nobody will use null specifically, if they use > it, they use it as a placeholder for generic code capable of handling > any muxer. And since it is generic code, it will handle any codec. > > But anyway, you are reading this commit wrongly. It does not remove > support for rawvideo in null, rawvideo is still supported in null, as > any other codec. This commit only changes the default codec. Thank you, I had never realized this in all the years I (thought I) had to specify --enable-decoder=rawvideo,wrapped_avframe... Carl Eugen
Carl Eugen Hoyos (12020-04-06): > Thank you, I had never realized this in all the years I (thought I) had > to specify --enable-decoder=rawvideo,wrapped_avframe... I am not sure exactly what you are saying. You would need the corresponding encoder, of course, whatever it is. And if it's not the default, specifying it would be necessary. Regards,
Am Mo., 6. Apr. 2020 um 23:13 Uhr schrieb Nicolas George <george@nsup.org>: > > Carl Eugen Hoyos (12020-04-06): > > Thank you, I had never realized this in all the years I (thought I) had > > to specify --enable-decoder=rawvideo,wrapped_avframe... > > I am not sure exactly what you are saying. I tried to explain that for many years, when I tested a regression and hit mentioned commit (which happens often), I had to recompile because I did not realize I could just force the rawvideo encoder. Carl Eugen
diff --git a/libavdevice/xv.c b/libavdevice/xv.c index c3ed2e48bd..2c5f1a4432 100644 --- a/libavdevice/xv.c +++ b/libavdevice/xv.c @@ -113,8 +113,8 @@ static int xv_write_header(AVFormatContext *s) if ( s->nb_streams > 1 || par->codec_type != AVMEDIA_TYPE_VIDEO - || par->codec_id != AV_CODEC_ID_RAWVIDEO) { - av_log(s, AV_LOG_ERROR, "Only supports one rawvideo stream\n"); + || par->codec_id != AV_CODEC_ID_WRAPPED_AVFRAME) { + av_log(s, AV_LOG_ERROR, "Only supports one wrapped avframe stream\n"); return AVERROR(EINVAL); } @@ -321,13 +321,8 @@ static int write_picture(AVFormatContext *s, uint8_t *input_data[4], static int xv_write_packet(AVFormatContext *s, AVPacket *pkt) { - AVCodecParameters *par = s->streams[0]->codecpar; - uint8_t *data[4]; - int linesize[4]; - - av_image_fill_arrays(data, linesize, pkt->data, par->format, - par->width, par->height, 1); - return write_picture(s, data, linesize); + AVFrame *frame = (AVFrame *)pkt->data; + return write_picture(s, frame->data, frame->linesize); } static int xv_write_frame(AVFormatContext *s, int stream_index, AVFrame **frame, @@ -375,7 +370,7 @@ AVOutputFormat ff_xv_muxer = { .long_name = NULL_IF_CONFIG_SMALL("XV (XVideo) output device"), .priv_data_size = sizeof(XVContext), .audio_codec = AV_CODEC_ID_NONE, - .video_codec = AV_CODEC_ID_RAWVIDEO, + .video_codec = AV_CODEC_ID_WRAPPED_AVFRAME, .write_header = xv_write_header, .write_packet = xv_write_packet, .write_uncoded_frame = xv_write_frame,
Signed-off-by: Marton Balint <cus@passwd.hu> --- libavdevice/xv.c | 15 +++++---------- 1 file changed, 5 insertions(+), 10 deletions(-)