| Message ID | 20170222224652.4505-2-cus@passwd.hu |
|---|---|
| State | Superseded |
| Headers | show |
Am 22.02.2017 um 23:46 schrieb Marton Balint: > diff --git a/doc/outdevs.texi b/doc/outdevs.texi > index e68653f..df41cc8 100644 > --- a/doc/outdevs.texi > +++ b/doc/outdevs.texi > @@ -131,8 +131,8 @@ and @code{--extra-ldflags}. > On Windows, you need to run the IDL files through @command{widl}. > > DeckLink is very picky about the formats it supports. Pixel format is always > -uyvy422, framerate and video size must be determined for your device with > -@command{-list_formats 1}. Audio sample rate is always 48 kHz. > +uyvy422, framerate, field order and video size must be determined for your > +device with @command{-list_formats 1}. Audio sample rate is always 48 kHz. The "@mode" syntax does not work for output devices. Let me add a patch, I will send it tonight. > diff --git a/libavdevice/decklink_common.cpp b/libavdevice/decklink_common.cpp > @@ -187,18 +201,21 @@ int ff_decklink_set_format(AVFormatContext *avctx, > BMDTimeValue bmd_tb_num, bmd_tb_den; > int bmd_width = mode->GetWidth(); > int bmd_height = mode->GetHeight(); > + BMDFieldDominance bmd_field_dominance = mode->GetFieldDominance(); > > mode->GetFrameRate(&bmd_tb_num, &bmd_tb_den); > AVRational mode_tb = av_make_q(bmd_tb_num, bmd_tb_den); > > - if ((bmd_width == width && bmd_height == height && > - !av_cmp_q(mode_tb, target_tb)) || i == num) { > + if ((bmd_width == width && > + bmd_height == height && > + !av_cmp_q(mode_tb, target_tb) && > + field_order_eq(field_order, bmd_field_dominance)) || i == num) { I worked on the same thing in the past days and had the following in mind: if ((!num && bmd_width == width && bmd_height == height && !av_cmp_q(mode_tb, target_tb) && field_order_eq(field_order, bmd_field_dominance)) || i == num) { So you can disable the "autodetect" when you explicitly specify a mode number. Matthias
On Thu, 23 Feb 2017, Matthias Hunstock wrote: > Am 22.02.2017 um 23:46 schrieb Marton Balint: > >> diff --git a/doc/outdevs.texi b/doc/outdevs.texi >> index e68653f..df41cc8 100644 >> --- a/doc/outdevs.texi >> +++ b/doc/outdevs.texi >> @@ -131,8 +131,8 @@ and @code{--extra-ldflags}. >> On Windows, you need to run the IDL files through @command{widl}. >> >> DeckLink is very picky about the formats it supports. Pixel format is always >> -uyvy422, framerate and video size must be determined for your device with >> -@command{-list_formats 1}. Audio sample rate is always 48 kHz. >> +uyvy422, framerate, field order and video size must be determined for your >> +device with @command{-list_formats 1}. Audio sample rate is always 48 kHz. > > > > The "@mode" syntax does not work for output devices. Let me add a patch, > I will send it tonight. Please don't. The @mode syntax is insane. You need a different @mode for different hardware to get the same output format. @mode may even be different across driver versions. Thankfully, the output device does not need it, with the field order patch, I really see no reason to support it. Regards, Marton
Am 23.02.2017 um 17:24 schrieb Marton Balint: >> The "@mode" syntax does not work for output devices. Let me add a patch, >> I will send it tonight. > > Please don't. The @mode syntax is insane. You need a different @mode for > different hardware to get the same output format. @mode may even be > different across driver versions. I know... the numbers are really a mess. What about supporting the mode strings? They are constant: bmdModeHD1080i50 = /* 'Hi50' */ 0x48693530 At least for capture this is IMO a benefit: ffmpeg -f decklink -i 'DeckLink Duo@Hi50' For playout... well if the automatic detection works and interlaced flag is always correct, fps is always correct (30 vs 29.97)... to be honest: if I'd do a setup, I'd sleep better with an explicit mode. But yeah, not those numbers. Matthias
On Thu, 23 Feb 2017, Matthias Hunstock wrote: > Am 23.02.2017 um 17:24 schrieb Marton Balint: >>> The "@mode" syntax does not work for output devices. Let me add a patch, >>> I will send it tonight. >> >> Please don't. The @mode syntax is insane. You need a different @mode for >> different hardware to get the same output format. @mode may even be >> different across driver versions. > > I know... the numbers are really a mess. > > What about supporting the mode strings? They are constant: > > bmdModeHD1080i50 = /* 'Hi50' */ 0x48693530 > > At least for capture this is IMO a benefit: > > ffmpeg -f decklink -i 'DeckLink Duo@Hi50' > > For playout... well if the automatic detection works and interlaced flag > is always correct, fps is always correct (30 vs 29.97)... to be honest: > if I'd do a setup, I'd sleep better with an explicit mode. But yeah, not > those numbers. Specifying the mode based on the decklink fourcc makes more sense. On the other hand, for output, you'd still have to check if the stream parameters are correct and if they are corresponding to the wanted mode. So if the user specify the mode with a decklink mode fourcc, you could only do a check like this (with &&): if ((bmd_width == width && bmd_height == height && !av_cmp_q(mode_tb, target_tb) && field_order_eq(field_order, bmd_field_dominance) && mode == bmd_mode) || i == num) { For input, using a decklink fourcc to specify a mode would be definitely useful. However, I don't like the @anything part in the device name. If you add this feature, I'd very much prefer a separate option of the device, 'format' maybe. I'd even deprecate @mode in the device name later. Regards, Marton
Am 24.02.2017 um 00:32 schrieb Marton Balint: > > On Thu, 23 Feb 2017, Matthias Hunstock wrote: > >> Am 23.02.2017 um 17:24 schrieb Marton Balint: >>>> The "@mode" syntax does not work for output devices. Let me add a >>>> patch, >>>> I will send it tonight. >>> >>> Please don't. The @mode syntax is insane. You need a different @mode for >>> different hardware to get the same output format. @mode may even be >>> different across driver versions. >> >> I know... the numbers are really a mess. >> >> What about supporting the mode strings? They are constant: >> >> bmdModeHD1080i50 = /* 'Hi50' */ 0x48693530 >> >> At least for capture this is IMO a benefit: >> >> ffmpeg -f decklink -i 'DeckLink Duo@Hi50' >> >> For playout... well if the automatic detection works and interlaced flag >> is always correct, fps is always correct (30 vs 29.97)... to be honest: >> if I'd do a setup, I'd sleep better with an explicit mode. But yeah, not >> those numbers. > > Specifying the mode based on the decklink fourcc makes more sense. > > On the other hand, for output, you'd still have to check if the stream > parameters are correct and if they are corresponding to the wanted mode. > So if the user specify the mode with a decklink mode fourcc, you could > only do a check like this (with &&): > > if ((bmd_width == width && > bmd_height == height && > !av_cmp_q(mode_tb, target_tb) && > field_order_eq(field_order, bmd_field_dominance) && > mode == bmd_mode) || i == num) { This disallows "hacks" like doing interlaced playout of progressive content or playout 30 fps with 29.97fps. Maybe it should just check widht and height for a given mode? Like: if ((bmd_width == width && bmd_height == height && mode == bmd_mode) || (!mode && !av_cmp_q(mode_tb, target_tb) && bmd_width == width && bmd_height == height && field_order_eq(field_order, bmd_field_dominance)) || i == num /* deprecated */) { (ORs seperated for readability) Or would you suggest that a user forces such situation e.g. with -vf setfields or -r ? > For input, using a decklink fourcc to specify a mode would be definitely > useful. > > However, I don't like the @anything part in the device name. If you add > this feature, I'd very much prefer a separate option of the device, > 'format' maybe. I'd even deprecate @mode in the device name later. ACK For the time being, I'd like to see your patches pushed since they improve the situation and the patchset does lots of other things. Matthias
On Fri, 24 Feb 2017, Matthias Hunstock wrote: > Am 24.02.2017 um 00:32 schrieb Marton Balint: >> >> On Thu, 23 Feb 2017, Matthias Hunstock wrote: >> >>> Am 23.02.2017 um 17:24 schrieb Marton Balint: >>>>> The "@mode" syntax does not work for output devices. Let me add a >>>>> patch, >>>>> I will send it tonight. >>>> >>>> Please don't. The @mode syntax is insane. You need a different @mode for >>>> different hardware to get the same output format. @mode may even be >>>> different across driver versions. >>> >>> I know... the numbers are really a mess. >>> >>> What about supporting the mode strings? They are constant: >>> >>> bmdModeHD1080i50 = /* 'Hi50' */ 0x48693530 >>> >>> At least for capture this is IMO a benefit: >>> >>> ffmpeg -f decklink -i 'DeckLink Duo@Hi50' >>> >>> For playout... well if the automatic detection works and interlaced flag >>> is always correct, fps is always correct (30 vs 29.97)... to be honest: >>> if I'd do a setup, I'd sleep better with an explicit mode. But yeah, not >>> those numbers. >> >> Specifying the mode based on the decklink fourcc makes more sense. >> >> On the other hand, for output, you'd still have to check if the stream >> parameters are correct and if they are corresponding to the wanted mode. >> So if the user specify the mode with a decklink mode fourcc, you could >> only do a check like this (with &&): >> >> if ((bmd_width == width && >> bmd_height == height && >> !av_cmp_q(mode_tb, target_tb) && >> field_order_eq(field_order, bmd_field_dominance) && >> mode == bmd_mode) || i == num) { > > > This disallows "hacks" like doing interlaced playout of progressive > content or playout 30 fps with 29.97fps. > > Maybe it should just check widht and height for a given mode? Like: > > if ((bmd_width == width && > bmd_height == height && > mode == bmd_mode) > || > (!mode && > !av_cmp_q(mode_tb, target_tb) && > bmd_width == width && > bmd_height == height && > field_order_eq(field_order, bmd_field_dominance)) > || > i == num /* deprecated */) { > > > (ORs seperated for readability) > > Or would you suggest that a user forces such situation e.g. with -vf > setfields or -r ? Yeah, I think that is somewhat cleaner. > > >> For input, using a decklink fourcc to specify a mode would be definitely >> useful. >> >> However, I don't like the @anything part in the device name. If you add >> this feature, I'd very much prefer a separate option of the device, >> 'format' maybe. I'd even deprecate @mode in the device name later. > > ACK > > For the time being, I'd like to see your patches pushed since they > improve the situation and the patchset does lots of other things. > Ok, will push in 1-2 days. Regards, Marton
On Sat, 25 Feb 2017, Marton Balint wrote: [...] > > Ok, will push in 1-2 days. Series applied. Regards, Marton
diff --git a/doc/outdevs.texi b/doc/outdevs.texi index e68653f..df41cc8 100644 --- a/doc/outdevs.texi +++ b/doc/outdevs.texi @@ -131,8 +131,8 @@ and @code{--extra-ldflags}. On Windows, you need to run the IDL files through @command{widl}. DeckLink is very picky about the formats it supports. Pixel format is always -uyvy422, framerate and video size must be determined for your device with -@command{-list_formats 1}. Audio sample rate is always 48 kHz. +uyvy422, framerate, field order and video size must be determined for your +device with @command{-list_formats 1}. Audio sample rate is always 48 kHz. @subsection Options diff --git a/libavdevice/decklink_common.cpp b/libavdevice/decklink_common.cpp index a3bc58d..92b4545 100644 --- a/libavdevice/decklink_common.cpp +++ b/libavdevice/decklink_common.cpp @@ -130,9 +130,23 @@ static int decklink_select_input(AVFormatContext *avctx, BMDDeckLinkConfiguratio return 0; } +static DECKLINK_BOOL field_order_eq(enum AVFieldOrder field_order, BMDFieldDominance bmd_field_order) +{ + if (field_order == AV_FIELD_UNKNOWN) + return true; + if (field_order == AV_FIELD_TT && bmd_field_order == bmdUpperFieldFirst) + return true; + if (field_order == AV_FIELD_BB && bmd_field_order == bmdLowerFieldFirst) + return true; + if (field_order == AV_FIELD_PROGRESSIVE && (bmd_field_order == bmdProgressiveFrame || bmd_field_order == bmdProgressiveSegmentedFrame)) + return true; + return false; +} + int ff_decklink_set_format(AVFormatContext *avctx, int width, int height, int tb_num, int tb_den, + enum AVFieldOrder field_order, decklink_direction_t direction, int num) { struct decklink_cctx *cctx = (struct decklink_cctx *)avctx->priv_data; @@ -143,8 +157,8 @@ int ff_decklink_set_format(AVFormatContext *avctx, int i = 1; HRESULT res; - av_log(avctx, AV_LOG_DEBUG, "Trying to find mode for frame size %dx%d, frame timing %d/%d, direction %d, mode number %d\n", - width, height, tb_num, tb_den, direction, num); + av_log(avctx, AV_LOG_DEBUG, "Trying to find mode for frame size %dx%d, frame timing %d/%d, field order %d, direction %d, mode number %d\n", + width, height, tb_num, tb_den, field_order, direction, num); if (ctx->duplex_mode) { DECKLINK_BOOL duplex_supported = false; @@ -187,18 +201,21 @@ int ff_decklink_set_format(AVFormatContext *avctx, BMDTimeValue bmd_tb_num, bmd_tb_den; int bmd_width = mode->GetWidth(); int bmd_height = mode->GetHeight(); + BMDFieldDominance bmd_field_dominance = mode->GetFieldDominance(); mode->GetFrameRate(&bmd_tb_num, &bmd_tb_den); AVRational mode_tb = av_make_q(bmd_tb_num, bmd_tb_den); - if ((bmd_width == width && bmd_height == height && - !av_cmp_q(mode_tb, target_tb)) || i == num) { + if ((bmd_width == width && + bmd_height == height && + !av_cmp_q(mode_tb, target_tb) && + field_order_eq(field_order, bmd_field_dominance)) || i == num) { ctx->bmd_mode = mode->GetDisplayMode(); ctx->bmd_width = bmd_width; ctx->bmd_height = bmd_height; ctx->bmd_tb_den = bmd_tb_den; ctx->bmd_tb_num = bmd_tb_num; - ctx->bmd_field_dominance = mode->GetFieldDominance(); + ctx->bmd_field_dominance = bmd_field_dominance; av_log(avctx, AV_LOG_INFO, "Found Decklink mode %d x %d with rate %.2f%s\n", bmd_width, bmd_height, 1/av_q2d(mode_tb), (ctx->bmd_field_dominance==bmdLowerFieldFirst || ctx->bmd_field_dominance==bmdUpperFieldFirst)?"(i)":""); @@ -230,7 +247,7 @@ int ff_decklink_set_format(AVFormatContext *avctx, } int ff_decklink_set_format(AVFormatContext *avctx, decklink_direction_t direction, int num) { - return ff_decklink_set_format(avctx, 0, 0, 0, 0, direction, num); + return ff_decklink_set_format(avctx, 0, 0, 0, 0, AV_FIELD_UNKNOWN, direction, num); } int ff_decklink_list_devices(AVFormatContext *avctx) diff --git a/libavdevice/decklink_common.h b/libavdevice/decklink_common.h index bfa2b08..4753287 100644 --- a/libavdevice/decklink_common.h +++ b/libavdevice/decklink_common.h @@ -128,7 +128,7 @@ static const BMDVideoConnection decklink_video_connection_map[] = { }; HRESULT ff_decklink_get_display_name(IDeckLink *This, const char **displayName); -int ff_decklink_set_format(AVFormatContext *avctx, int width, int height, int tb_num, int tb_den, decklink_direction_t direction = DIRECTION_OUT, int num = 0); +int ff_decklink_set_format(AVFormatContext *avctx, int width, int height, int tb_num, int tb_den, enum AVFieldOrder field_order, decklink_direction_t direction = DIRECTION_OUT, int num = 0); int ff_decklink_set_format(AVFormatContext *avctx, decklink_direction_t direction, int num); int ff_decklink_list_devices(AVFormatContext *avctx); int ff_decklink_list_formats(AVFormatContext *avctx, decklink_direction_t direction = DIRECTION_OUT); diff --git a/libavdevice/decklink_enc.cpp b/libavdevice/decklink_enc.cpp index 944e9d3..8fb6a9c 100644 --- a/libavdevice/decklink_enc.cpp +++ b/libavdevice/decklink_enc.cpp @@ -105,8 +105,8 @@ static int decklink_setup_video(AVFormatContext *avctx, AVStream *st) return -1; } if (ff_decklink_set_format(avctx, c->width, c->height, - st->time_base.num, st->time_base.den)) { - av_log(avctx, AV_LOG_ERROR, "Unsupported video size or framerate!" + st->time_base.num, st->time_base.den, c->field_order)) { + av_log(avctx, AV_LOG_ERROR, "Unsupported video size, framerate or field order!" " Check available formats with -list_formats 1.\n"); return -1; } diff --git a/libavdevice/version.h b/libavdevice/version.h index 94f5659..ceec2d4 100644 --- a/libavdevice/version.h +++ b/libavdevice/version.h @@ -29,7 +29,7 @@ #define LIBAVDEVICE_VERSION_MAJOR 57 #define LIBAVDEVICE_VERSION_MINOR 2 -#define LIBAVDEVICE_VERSION_MICRO 100 +#define LIBAVDEVICE_VERSION_MICRO 101 #define LIBAVDEVICE_VERSION_INT AV_VERSION_INT(LIBAVDEVICE_VERSION_MAJOR, \ LIBAVDEVICE_VERSION_MINOR, \
Signed-off-by: Marton Balint <cus@passwd.hu> --- doc/outdevs.texi | 4 ++-- libavdevice/decklink_common.cpp | 29 +++++++++++++++++++++++------ libavdevice/decklink_common.h | 2 +- libavdevice/decklink_enc.cpp | 4 ++-- libavdevice/version.h | 2 +- 5 files changed, 29 insertions(+), 12 deletions(-)