| Message ID | 20200624190958.47004-1-rcombs@rcombs.me |
|---|---|
| State | New |
| Headers | show |
| Series | [FFmpeg-devel] lavu: make AV_TIME_BASE_Q work in C++ code | expand |
| Context | Check | Description |
|---|---|---|
| andriy/default | pending | |
| andriy/make | success | Make finished |
| andriy/make_fate | success | Make fate finished |
Am Mi., 24. Juni 2020 um 21:10 Uhr schrieb rcombs <rcombs@rcombs.me>: > > --- > libavutil/avutil.h | 4 ++++ > 1 file changed, 4 insertions(+) > > diff --git a/libavutil/avutil.h b/libavutil/avutil.h > index 4d633156d1..c11b33f466 100644 > --- a/libavutil/avutil.h > +++ b/libavutil/avutil.h > @@ -257,7 +257,11 @@ const char *av_get_media_type_string(enum AVMediaType media_type); > * Internal time base represented as fractional value > */ > > +#ifdef __cplusplus > +#define AV_TIME_BASE_Q AVRational{1, AV_TIME_BASE} > +#else > #define AV_TIME_BASE_Q (AVRational){1, AV_TIME_BASE} > +#endif The problem with this patch is that it gives C++ users of the libraries the feeling that we would care about them and that we would indeed fix issues. Given that the contrary is true (at least in the past) and that we do not do any specific C++ testing, I believe it is better to let users work around this issue (from their point of you). Carl Eugen
ons 2020-06-24 klockan 21:22 +0200 skrev Carl Eugen Hoyos: > Am Mi., 24. Juni 2020 um 21:10 Uhr schrieb rcombs <rcombs@rcombs.me>: > > --- > > libavutil/avutil.h | 4 ++++ > > 1 file changed, 4 insertions(+) > > > > diff --git a/libavutil/avutil.h b/libavutil/avutil.h > > index 4d633156d1..c11b33f466 100644 > > --- a/libavutil/avutil.h > > +++ b/libavutil/avutil.h > > @@ -257,7 +257,11 @@ const char *av_get_media_type_string(enum AVMediaType media_type); > > * Internal time base represented as fractional value > > */ > > > > +#ifdef __cplusplus > > +#define AV_TIME_BASE_Q AVRational{1, AV_TIME_BASE} > > +#else > > #define AV_TIME_BASE_Q (AVRational){1, AV_TIME_BASE} > > +#endif > > The problem with this patch is that it gives C++ users of the libraries > the feeling that we would care about them and that we would indeed > fix issues. > Given that the contrary is true (at least in the past) and that we do > not do any specific C++ testing, I believe it is better to let users > work around this issue (from their point of you). Maybe we should put in a #warning to that effect? Inside an #ifdef __cplusplus /Tomas
On Fri, Jun 26, 2020 at 11:31 AM Tomas Härdin <tjoppen@acc.umu.se> wrote: > > ons 2020-06-24 klockan 21:22 +0200 skrev Carl Eugen Hoyos: > > Am Mi., 24. Juni 2020 um 21:10 Uhr schrieb rcombs <rcombs@rcombs.me>: > > > --- > > > libavutil/avutil.h | 4 ++++ > > > 1 file changed, 4 insertions(+) > > > > > > diff --git a/libavutil/avutil.h b/libavutil/avutil.h > > > index 4d633156d1..c11b33f466 100644 > > > --- a/libavutil/avutil.h > > > +++ b/libavutil/avutil.h > > > @@ -257,7 +257,11 @@ const char *av_get_media_type_string(enum AVMediaType media_type); > > > * Internal time base represented as fractional value > > > */ > > > > > > +#ifdef __cplusplus > > > +#define AV_TIME_BASE_Q AVRational{1, AV_TIME_BASE} > > > +#else > > > #define AV_TIME_BASE_Q (AVRational){1, AV_TIME_BASE} > > > +#endif > > > > The problem with this patch is that it gives C++ users of the libraries > > the feeling that we would care about them and that we would indeed > > fix issues. > > Given that the contrary is true (at least in the past) and that we do > > not do any specific C++ testing, I believe it is better to let users > > work around this issue (from their point of you). > > Maybe we should put in a #warning to that effect? Inside an #ifdef > __cplusplus > Now that would just be annoying. 99% of the headers work just fine in C++, spewing a warning at us for using them would not serve anything. - Hendrik
fre 2020-06-26 klockan 11:36 +0200 skrev Hendrik Leppkes: > On Fri, Jun 26, 2020 at 11:31 AM Tomas Härdin <tjoppen@acc.umu.se> wrote: > > ons 2020-06-24 klockan 21:22 +0200 skrev Carl Eugen Hoyos: > > > Am Mi., 24. Juni 2020 um 21:10 Uhr schrieb rcombs <rcombs@rcombs.me>: > > > > --- > > > > libavutil/avutil.h | 4 ++++ > > > > 1 file changed, 4 insertions(+) > > > > > > > > diff --git a/libavutil/avutil.h b/libavutil/avutil.h > > > > index 4d633156d1..c11b33f466 100644 > > > > --- a/libavutil/avutil.h > > > > +++ b/libavutil/avutil.h > > > > @@ -257,7 +257,11 @@ const char *av_get_media_type_string(enum AVMediaType media_type); > > > > * Internal time base represented as fractional value > > > > */ > > > > > > > > +#ifdef __cplusplus > > > > +#define AV_TIME_BASE_Q AVRational{1, AV_TIME_BASE} > > > > +#else > > > > #define AV_TIME_BASE_Q (AVRational){1, AV_TIME_BASE} > > > > +#endif > > > > > > The problem with this patch is that it gives C++ users of the libraries > > > the feeling that we would care about them and that we would indeed > > > fix issues. > > > Given that the contrary is true (at least in the past) and that we do > > > not do any specific C++ testing, I believe it is better to let users > > > work around this issue (from their point of you). > > > > Maybe we should put in a #warning to that effect? Inside an #ifdef > > __cplusplus > > > > Now that would just be annoying. 99% of the headers work just fine in > C++, spewing a warning at us for using them would not serve anything. Well, if you volunteer to maintain C++ support then sure. Most devs in this project are C guys. Admittedly this is a bit of a Måns-ism, but he's no longer involved so.. I don't think anyone is principally opposed, it's just a question of where to spend developer effort. /Tomas
Am Sa., 27. Juni 2020 um 11:47 Uhr schrieb Tomas Härdin <tjoppen@acc.umu.se>: > > fre 2020-06-26 klockan 11:36 +0200 skrev Hendrik Leppkes: > > On Fri, Jun 26, 2020 at 11:31 AM Tomas Härdin <tjoppen@acc.umu.se> wrote: > > > ons 2020-06-24 klockan 21:22 +0200 skrev Carl Eugen Hoyos: > > > > Am Mi., 24. Juni 2020 um 21:10 Uhr schrieb rcombs <rcombs@rcombs.me>: > > > > > --- > > > > > libavutil/avutil.h | 4 ++++ > > > > > 1 file changed, 4 insertions(+) > > > > > > > > > > diff --git a/libavutil/avutil.h b/libavutil/avutil.h > > > > > index 4d633156d1..c11b33f466 100644 > > > > > --- a/libavutil/avutil.h > > > > > +++ b/libavutil/avutil.h > > > > > @@ -257,7 +257,11 @@ const char *av_get_media_type_string(enum AVMediaType media_type); > > > > > * Internal time base represented as fractional value > > > > > */ > > > > > > > > > > +#ifdef __cplusplus > > > > > +#define AV_TIME_BASE_Q AVRational{1, AV_TIME_BASE} > > > > > +#else > > > > > #define AV_TIME_BASE_Q (AVRational){1, AV_TIME_BASE} > > > > > +#endif > > > > > > > > The problem with this patch is that it gives C++ users of the libraries > > > > the feeling that we would care about them and that we would indeed > > > > fix issues. > > > > Given that the contrary is true (at least in the past) and that we do > > > > not do any specific C++ testing, I believe it is better to let users > > > > work around this issue (from their point of you). > > > > > > Maybe we should put in a #warning to that effect? Inside an #ifdef > > > __cplusplus > > > > > > > Now that would just be annoying. 99% of the headers work just fine in > > C++, spewing a warning at us for using them would not serve anything. > > Well, if you volunteer to maintain C++ support then sure. Most devs in > this project are C guys. Admittedly this is a bit of a Måns-ism, but > he's no longer involved so.. I don't think anyone is principally > opposed, it's just a question of where to spend developer effort. Not sure why you believe we need a warning: Iiuc, currently AV_TIME_BASE_Q cannot be used from C++ programs, any C++ developer will immediately be made aware of this, no? Carl Eugen
lör 2020-06-27 klockan 11:51 +0200 skrev Carl Eugen Hoyos: > Am Sa., 27. Juni 2020 um 11:47 Uhr schrieb Tomas Härdin <tjoppen@acc.umu.se>: > > fre 2020-06-26 klockan 11:36 +0200 skrev Hendrik Leppkes: > > > On Fri, Jun 26, 2020 at 11:31 AM Tomas Härdin <tjoppen@acc.umu.se> wrote: > > > > ons 2020-06-24 klockan 21:22 +0200 skrev Carl Eugen Hoyos: > > > > > Am Mi., 24. Juni 2020 um 21:10 Uhr schrieb rcombs <rcombs@rcombs.me>: > > > > > > --- > > > > > > libavutil/avutil.h | 4 ++++ > > > > > > 1 file changed, 4 insertions(+) > > > > > > > > > > > > diff --git a/libavutil/avutil.h b/libavutil/avutil.h > > > > > > index 4d633156d1..c11b33f466 100644 > > > > > > --- a/libavutil/avutil.h > > > > > > +++ b/libavutil/avutil.h > > > > > > @@ -257,7 +257,11 @@ const char *av_get_media_type_string(enum AVMediaType media_type); > > > > > > * Internal time base represented as fractional value > > > > > > */ > > > > > > > > > > > > +#ifdef __cplusplus > > > > > > +#define AV_TIME_BASE_Q AVRational{1, AV_TIME_BASE} > > > > > > +#else > > > > > > #define AV_TIME_BASE_Q (AVRational){1, AV_TIME_BASE} > > > > > > +#endif > > > > > > > > > > The problem with this patch is that it gives C++ users of the libraries > > > > > the feeling that we would care about them and that we would indeed > > > > > fix issues. > > > > > Given that the contrary is true (at least in the past) and that we do > > > > > not do any specific C++ testing, I believe it is better to let users > > > > > work around this issue (from their point of you). > > > > > > > > Maybe we should put in a #warning to that effect? Inside an #ifdef > > > > __cplusplus > > > > > > > > > > Now that would just be annoying. 99% of the headers work just fine in > > > C++, spewing a warning at us for using them would not serve anything. > > > > Well, if you volunteer to maintain C++ support then sure. Most devs in > > this project are C guys. Admittedly this is a bit of a Måns-ism, but > > he's no longer involved so.. I don't think anyone is principally > > opposed, it's just a question of where to spend developer effort. > > Not sure why you believe we need a warning: > Iiuc, currently AV_TIME_BASE_Q cannot be used from C++ programs, > any C++ developer will immediately be made aware of this, no? True, but it's generating threads on the list like this one on a regular basis. /Tomas
On 6/30/20, Tomas Härdin <tjoppen@acc.umu.se> wrote: > lör 2020-06-27 klockan 11:51 +0200 skrev Carl Eugen Hoyos: >> Am Sa., 27. Juni 2020 um 11:47 Uhr schrieb Tomas Härdin >> <tjoppen@acc.umu.se>: >> > fre 2020-06-26 klockan 11:36 +0200 skrev Hendrik Leppkes: >> > > On Fri, Jun 26, 2020 at 11:31 AM Tomas Härdin <tjoppen@acc.umu.se> >> > > wrote: >> > > > ons 2020-06-24 klockan 21:22 +0200 skrev Carl Eugen Hoyos: >> > > > > Am Mi., 24. Juni 2020 um 21:10 Uhr schrieb rcombs >> > > > > <rcombs@rcombs.me>: >> > > > > > --- >> > > > > > libavutil/avutil.h | 4 ++++ >> > > > > > 1 file changed, 4 insertions(+) >> > > > > > >> > > > > > diff --git a/libavutil/avutil.h b/libavutil/avutil.h >> > > > > > index 4d633156d1..c11b33f466 100644 >> > > > > > --- a/libavutil/avutil.h >> > > > > > +++ b/libavutil/avutil.h >> > > > > > @@ -257,7 +257,11 @@ const char *av_get_media_type_string(enum >> > > > > > AVMediaType media_type); >> > > > > > * Internal time base represented as fractional value >> > > > > > */ >> > > > > > >> > > > > > +#ifdef __cplusplus >> > > > > > +#define AV_TIME_BASE_Q AVRational{1, AV_TIME_BASE} >> > > > > > +#else >> > > > > > #define AV_TIME_BASE_Q (AVRational){1, AV_TIME_BASE} >> > > > > > +#endif >> > > > > >> > > > > The problem with this patch is that it gives C++ users of the >> > > > > libraries >> > > > > the feeling that we would care about them and that we would indeed >> > > > > fix issues. >> > > > > Given that the contrary is true (at least in the past) and that we >> > > > > do >> > > > > not do any specific C++ testing, I believe it is better to let >> > > > > users >> > > > > work around this issue (from their point of you). >> > > > >> > > > Maybe we should put in a #warning to that effect? Inside an #ifdef >> > > > __cplusplus >> > > > >> > > >> > > Now that would just be annoying. 99% of the headers work just fine in >> > > C++, spewing a warning at us for using them would not serve anything. >> > >> > Well, if you volunteer to maintain C++ support then sure. Most devs in >> > this project are C guys. Admittedly this is a bit of a Måns-ism, but >> > he's no longer involved so.. I don't think anyone is principally >> > opposed, it's just a question of where to spend developer effort. >> >> Not sure why you believe we need a warning: >> Iiuc, currently AV_TIME_BASE_Q cannot be used from C++ programs, >> any C++ developer will immediately be made aware of this, no? > > True, but it's generating threads on the list like this one on a > regular basis. > So we should suffer instead by applying this patch? > /Tomas > > _______________________________________________ > ffmpeg-devel mailing list > ffmpeg-devel@ffmpeg.org > https://ffmpeg.org/mailman/listinfo/ffmpeg-devel > > To unsubscribe, visit link above, or email > ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".
On Tue, Jun 30, 2020 at 12:00:24PM +0200, Paul B Mahol wrote:
> So we should suffer instead by applying this patch?
The thousands of projects that include FFmpeg from C++, directly or
indirectly, suggests that the amount of suffering on FFmpeg's part
by supporting inclusion from C++ is not going to be large.
If you add some #ifndef __cplusplus extern "C" ... on each header and then
apply the given patch, I would be surprised if this creates a large
maintenance burden on your behalf.
/* Steinar */
diff --git a/libavutil/avutil.h b/libavutil/avutil.h index 4d633156d1..c11b33f466 100644 --- a/libavutil/avutil.h +++ b/libavutil/avutil.h @@ -257,7 +257,11 @@ const char *av_get_media_type_string(enum AVMediaType media_type); * Internal time base represented as fractional value */ +#ifdef __cplusplus +#define AV_TIME_BASE_Q AVRational{1, AV_TIME_BASE} +#else #define AV_TIME_BASE_Q (AVRational){1, AV_TIME_BASE} +#endif /** * @}