Re: [FFmpeg-devel] [PATCH 0/4] ffplay and lavd SDL2 set
On 15/09/2016 11:11, Thomas Volkert wrote: [...] Yes, it is quite a bit of unnecessary overhead, but it's much cleaner than #ifdeffery in a single source file. In general, I don't like the idea of having 2 source files with almost the same content. This usually indicates to me that the code structure could be improved. But ...you said you take over the maintenance which is needed. So, I am fine with this step as long as the old file gets dropped soon (in terms of months instead of years). (And I also don't like a flood of #ifdefs. ;) ) Yes, ffplay_sdl1.c will get dropped as soon as support for Debian old-stable is dropped. Also, to change ffplay.c for most fixes as you'd have to reflect the change on either side of the #ifdefs, so you don't gain anything from not splitting the sources. I agree for SDL related patches. This apparently has to be done for SDL1 as well as SDL2. However, for other patches you have to do copy+paste to get them in both ffplay source files. But see above .. I am fine if you compensate this. That's fine then -- Josh ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH 0/4] ffplay and lavd SDL2 set
On 15.09.2016 08:55, Josh de Kock wrote: On 14/09/2016 23:44, Thomas Volkert wrote: On 15.09.2016 00:27, Josh de Kock wrote: Hi, Resending this set with ffplay now having two versions, a SDL2 and a SDL1 version. I've integrated all comments up until now (hopefully). Josh Josh de Kock (3): lavd: Add SDL2 output device ffplay: make copy for SDL1 MAINTAINERS: update my entries Marton Balint (1): ffplay: add SDL2 support MAINTAINERS | 4 +- Makefile | 1 + configure | 35 ++- ffplay.c | 599 +++--- ffplay.c => ffplay_sdl1.c | 1 - Maintaining two versions of ffplay sounds as too much overhead to me. Is it not possible to add some more code abstraction or even some simple #ifdef constructs to support SDL1 as well as SDL2 in only one ffplay version in parallel? So, the actually used SDL version could be selected during the configure step. After some months, the (hopefully) deprecated SDL1 code could be dropped... Yes, it is quite a bit of unnecessary overhead, but it's much cleaner than #ifdeffery in a single source file. In general, I don't like the idea of having 2 source files with almost the same content. This usually indicates to me that the code structure could be improved. But ...you said you take over the maintenance which is needed. So, I am fine with this step as long as the old file gets dropped soon (in terms of months instead of years). (And I also don't like a flood of #ifdefs. ;) ) Also, to change ffplay.c for most fixes as you'd have to reflect the change on either side of the #ifdefs, so you don't gain anything from not splitting the sources. I agree for SDL related patches. This apparently has to be done for SDL1 as well as SDL2. However, for other patches you have to do copy+paste to get them in both ffplay source files. But see above .. I am fine if you compensate this. Best regards, Thomas. ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH 0/4] ffplay and lavd SDL2 set
On 14/09/2016 23:44, Thomas Volkert wrote: On 15.09.2016 00:27, Josh de Kock wrote: Hi, Resending this set with ffplay now having two versions, a SDL2 and a SDL1 version. I've integrated all comments up until now (hopefully). Josh Josh de Kock (3): lavd: Add SDL2 output device ffplay: make copy for SDL1 MAINTAINERS: update my entries Marton Balint (1): ffplay: add SDL2 support MAINTAINERS | 4 +- Makefile | 1 + configure | 35 ++- ffplay.c | 599 +++--- ffplay.c => ffplay_sdl1.c | 1 - Maintaining two versions of ffplay sounds as too much overhead to me. Is it not possible to add some more code abstraction or even some simple #ifdef constructs to support SDL1 as well as SDL2 in only one ffplay version in parallel? So, the actually used SDL version could be selected during the configure step. After some months, the (hopefully) deprecated SDL1 code could be dropped... Yes, it is quite a bit of unnecessary overhead, but it's much cleaner than #ifdeffery in a single source file. Also, to change ffplay.c for most fixes as you'd have to reflect the change on either side of the #ifdefs, so you don't gain anything from not splitting the sources. -- Josh ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH 0/4] ffplay and lavd SDL2 set
On 9/14/2016 7:44 PM, Thomas Volkert wrote: > On 15.09.2016 00:27, Josh de Kock wrote: >> Hi, >> >> Resending this set with ffplay now having two versions, a SDL2 and a >> SDL1 version. I've integrated all comments up until now (hopefully). >> >> Josh >> >> Josh de Kock (3): >>lavd: Add SDL2 output device >>ffplay: make copy for SDL1 >>MAINTAINERS: update my entries >> >> Marton Balint (1): >>ffplay: add SDL2 support >> >> MAINTAINERS | 4 +- >> Makefile | 1 + >> configure | 35 ++- >> ffplay.c | 599 >> +++--- >> ffplay.c => ffplay_sdl1.c | 1 - >> > > Maintaining two versions of ffplay sounds as too much overhead to me. Is it > not possible to add some more code abstraction or even some simple #ifdef > constructs to support SDL1 as well as SDL2 in only one ffplay version in > parallel? > So, the actually used SDL version could be selected during the configure > step. After some months, the (hopefully) deprecated SDL1 code could be > dropped... > > Best regards, > Thomas. This is not two separate ffplay programs. It's exactly what you said except using two separate source files instead of #ifdeffery in a single source file. Look at patches 2 and 3 in the set. ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH 0/4] ffplay and lavd SDL2 set
On 15.09.2016 00:27, Josh de Kock wrote: Hi, Resending this set with ffplay now having two versions, a SDL2 and a SDL1 version. I've integrated all comments up until now (hopefully). Josh Josh de Kock (3): lavd: Add SDL2 output device ffplay: make copy for SDL1 MAINTAINERS: update my entries Marton Balint (1): ffplay: add SDL2 support MAINTAINERS | 4 +- Makefile | 1 + configure | 35 ++- ffplay.c | 599 +++--- ffplay.c => ffplay_sdl1.c | 1 - Maintaining two versions of ffplay sounds as too much overhead to me. Is it not possible to add some more code abstraction or even some simple #ifdef constructs to support SDL1 as well as SDL2 in only one ffplay version in parallel? So, the actually used SDL version could be selected during the configure step. After some months, the (hopefully) deprecated SDL1 code could be dropped... Best regards, Thomas. ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
[FFmpeg-devel] [PATCH 0/4] ffplay and lavd SDL2 set
Hi, Resending this set with ffplay now having two versions, a SDL2 and a SDL1 version. I've integrated all comments up until now (hopefully). Josh Josh de Kock (3): lavd: Add SDL2 output device ffplay: make copy for SDL1 MAINTAINERS: update my entries Marton Balint (1): ffplay: add SDL2 support MAINTAINERS | 4 +- Makefile | 1 + configure | 35 ++- ffplay.c | 599 +++--- ffplay.c => ffplay_sdl1.c | 1 - libavdevice/Makefile | 1 + libavdevice/alldevices.c | 1 + libavdevice/sdl2.c| 377 + 8 files changed, 665 insertions(+), 354 deletions(-) copy ffplay.c => ffplay_sdl1.c (99%) create mode 100644 libavdevice/sdl2.c -- 2.7.4 (Apple Git-66) ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
