Skip to content

video: add a constructor with API preference parameter for VideoWriter#8920

Merged
opencv-pushbot merged 1 commit intoopencv:masterfrom
sovrasov:video_vriter_ext
Jun 19, 2017
Merged

video: add a constructor with API preference parameter for VideoWriter#8920
opencv-pushbot merged 1 commit intoopencv:masterfrom
sovrasov:video_vriter_ext

Conversation

@sovrasov
Copy link
Copy Markdown
Contributor

@sovrasov sovrasov commented Jun 15, 2017

resolves #8859

*/
CVAPI(CvVideoWriter*) cvCreateVideoWriterWithPreference(int api, const char* filename, int fourcc,
double fps, CvSize frame_size,
int is_color CV_DEFAULT(1));
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Probably we should not add new exported cv* functions. Please make it local "static".

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've moved it to the precomp.hpp where other similar functions are declared.

default:
//exit if the specified API is unavaliable
if (apiPreference != CV_CAP_ANY) break;
case(CV_CAP_FFMPEG):
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

BTW, Why "case" values are in brackets?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Don't know, just a habit.

@alalek
Copy link
Copy Markdown
Member

alalek commented Jun 15, 2017

Looks good to me. Well done! 👍

{
CV_INSTRUMENT_REGION()

if (isOpened()) release();
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We should propagate apiPreference to this constructor too.

CvVideoWriter *result = 0;

if(!fourcc || !fps)
TRY_OPEN(result, cvCreateVideoWriter_Images(filename))
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe we could define additional API IDs for built-in MotionJPEG and Images backends? Or maybe we should add new set of IDs for VideoWriter specifically: WRI_ANY, WRI_FFMPEG, etc.? What do you think?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've defined CAP_OCV_MJPEG and we already have CAP_IMAGES.

default:
//exit if the specified API is unavaliable
if (apiPreference != CV_CAP_ANY) break;
case(CV_CAP_FFMPEG):
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In my opinion wrapping whole case blocks into #ifdef..#endif will look more clean:

#ifdef HAVE_FFMPEG
        case CV_CAP_FFMPEG:
            TRY_OPEN(result, cvCreateVideoWriter_FFMPEG_proxy (filename, fourcc, fps, frameSize, is_color))
            if (apiPreference != CV_CAP_ANY) break;
#endif

#ifdef HAVE_VFW
        case CV_CAP_VFW:
            TRY_OPEN(result, cvCreateVideoWriter_VFW(filename, fourcc, fps, frameSize, is_color))
            if (apiPreference != CV_CAP_ANY) break;
#endif

@mshabunin
Copy link
Copy Markdown
Contributor

👍

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Unable to use gstreamer videowriter

5 participants