Skip to content

Conversation

gcasa
Copy link
Member

@gcasa gcasa commented Mar 20, 2022

implementation for NSMovie and NSMovieView. The tests for this are here:
https://github.com/gcasa/NSMovie_test
https://github.com/gcasa/apps-videoplayer

@gcasa gcasa force-pushed the NSMovie_branch branch from 0ba3345 to 9664cd4 Compare May 25, 2025 20:29
@gcasa
Copy link
Member Author

gcasa commented May 26, 2025

Completely revamped and simplified.

@gcasa gcasa requested a review from rfm September 12, 2025 17:30
@gcasa
Copy link
Member Author

gcasa commented Sep 12, 2025

@rfm Please take a look at this one as well.

@gcasa gcasa requested a review from Copilot September 13, 2025 01:57
Copy link
Contributor

@Copilot Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR creates comprehensive NSMovie and NSMovieView implementation using FFmpeg as the underlying multimedia framework. The implementation adds video playback functionality with synchronized audio/video rendering and proper build system integration.

  • Adds complete NSMovie and NSMovieView classes with FFmpeg-based video/audio decoding capabilities
  • Implements GSMovieView as a subclass providing the actual playback functionality with audio-video synchronization
  • Integrates FFmpeg dependencies into the build system with proper configuration and library detection

Reviewed Changes

Copilot reviewed 17 out of 18 changed files in this pull request and generated 5 comments.

Show a summary per file
File Description
configure.ac Adds FFmpeg library detection and build configuration for NSMovie/NSMovieView
config.make.in Defines build variables for movie and sound library support
Source/NSSound.m Updates NSSound implementation with improved memory management
Source/NSMovieView.m Basic NSMovieView implementation with stubs for most functionality
Source/NSMovie.m NSMovie class implementation with data handling and codec integration
Source/GSMovieView.m Complete video player implementation with FFmpeg integration
Source/GSAudioPlayer.* Audio player class for synchronized audio playback
Source/GSAVUtils.* Utility functions for AVPacket/NSDictionary conversion
Headers/AppKit/NSMovie*.h Public API headers with comprehensive documentation

Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.

gcasa and others added 6 commits September 13, 2025 03:03
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
@gcasa
Copy link
Member Author

gcasa commented Sep 15, 2025

I think this is as close to done as I can get it. It works. I would like others to have the opportunity to look at and fix the code on this. Please review and approve if you can @rfm or @fredkiefer

@gcasa
Copy link
Member Author

gcasa commented Sep 24, 2025

I think this PR is ready as well, @fredkiefer / @rfm

@gcasa
Copy link
Member Author

gcasa commented Oct 2, 2025

@fredkiefer can you take one final look at this? I really have taken this as far as I can. It's not perfect, but it works. I would like to merge this so that others can inspect and improve the code.

@gcasa
Copy link
Member Author

gcasa commented Oct 4, 2025

Unless there is an objection I am going to merge this. I have taken it as far as I can. I want more people to be able to test it and improve it.

@fredkiefer
Copy link
Member

Just give me a few more hours. I am planing of doing a final review of this code tonight.

@gcasa
Copy link
Member Author

gcasa commented Oct 5, 2025

Okay. I didn't mean to rush you. I just wanted others to be able to take advantage of it.

Copy link
Member

@fredkiefer fredkiefer left a comment

Choose a reason for hiding this comment

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

I don't quite like the concepts behind the current implementation, but I don't see much harm in it. If you really want to, you may go ahead and merge this.

@gcasa
Copy link
Member Author

gcasa commented Oct 6, 2025

Merging this with fred's okay. Everyone please test this and let me know what I have done wrong with decoding, if anything or how I can improve my approach.

@gcasa gcasa merged commit 90d5118 into master Oct 6, 2025
4 checks passed
@gcasa gcasa deleted the NSMovie_branch branch October 6, 2025 02:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

2 participants