Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add method to FFmpegFrameGrabber to retrieve attached pic #1879

Merged
merged 8 commits into from
Oct 2, 2022
Merged

Add method to FFmpegFrameGrabber to retrieve attached pic #1879

merged 8 commits into from
Oct 2, 2022

Conversation

Tengyyy
Copy link
Contributor

@Tengyyy Tengyyy commented Aug 26, 2022

I couldn't get the code to compile due to some missing imports and other errors, but I hope this works. Can you by any chance test this?
#1876

@saudet
Copy link
Member

saudet commented Aug 26, 2022 via email

@Tengyyy
Copy link
Contributor Author

Tengyyy commented Aug 27, 2022

Im not sure I understand. Adding a flag to the frame object would help identify a frame with the attached pic. But I would still need to save a reference to the correct stream in the FFmpegFrameGrabber's startUnsafe() method, because finding the right stream is the bigger issue.

@saudet
Copy link
Member

saudet commented Aug 27, 2022

I see, what about having something like FrameGrabber.setVideoDisposition() to make the match?

@Tengyyy
Copy link
Contributor Author

Tengyyy commented Aug 27, 2022

Okay. So the way it would work is that you call setVideoDisposition(), then it finds the stream that contains an attached pic and sets the video stream to that. And then you would call frameGrabber.grabImage() like usual.
Is that what you mean?

@saudet
Copy link
Member

saudet commented Aug 27, 2022 via email

@saudet
Copy link
Member

saudet commented Sep 7, 2022

Please let me know if you have a better idea though. Thanks!

@Tengyyy
Copy link
Contributor Author

Tengyyy commented Sep 9, 2022

Sorry. I've been busy with school. Does this do the trick?

@saudet
Copy link
Member

saudet commented Sep 9, 2022

Something like that, yes, but add a property, just like for getVideoCodecName()/setVideoCodecName(), and use it in start().

@Tengyyy
Copy link
Contributor Author

Tengyyy commented Sep 16, 2022

What exactly should I do in the start() method?

@saudet
Copy link
Member

saudet commented Sep 16, 2022

Use the value of videoDisposition to set the videoStream, just like you did in your loop. Put that loop so it gets executed on start().

@Tengyyy
Copy link
Contributor Author

Tengyyy commented Sep 29, 2022

Did like you asked, but isn't the disposition only relevant for the FFmpegFrameGrabber and not all the other framegrabber types?

@saudet
Copy link
Member

saudet commented Sep 29, 2022

Sure, most of the properties there only apply to FFmpeg, but maybe in the future something else is going to become more popular and then that one will start using those properties.. Anyway, there is no disadvantage in providing an abstract class like this, only potential advantages, so let's just do it!

These changes look good. Did you test them to make sure they do what you need them to do?

@saudet saudet merged commit e47cb2f into bytedeco:master Oct 2, 2022
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.

None yet

2 participants