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.

Sign up for GitHub

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

Added validation at the start of recording. #1612

Merged
merged 2 commits into from
Nov 1, 2024
Merged

Added validation at the start of recording. #1612

Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
b48c1f7
Added validation at the start of recording.
shogo4405 Oct 29, 2024
099c48b
Add HKStreamRecorderTests
shogo4405 Oct 30, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
80 changes: 62 additions & 18 deletions Sources/HKStream/HKStreamRecorder.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,10 +20,16 @@
/// stream.addOutput(recorder)
/// ```
public actor HKStreamRecorder {
static let defaultPathExtension = "mp4"

/// The error domain codes.
public enum Error: Swift.Error {
/// An invalid internal stare.
case invalidState
/// The specified file already exists.
case fileAlreadyExists(outputURL: URL)
/// The specifiled file type is not supported.
case notSupportedFileType(pathExtension: String)
/// Failed to create the AVAssetWriter.
case failedToCreateAssetWriter(error: any Swift.Error)
/// Failed to create the AVAssetWriterInput.
Expand Down Expand Up @@ -59,16 +65,41 @@ public actor HKStreamRecorder {
}
}

enum SupportedFileType: String {
case mp4
case mov

var fileType: AVFileType {
switch self {
case .mp4:
return .mp4
case .mov:
return .mov
}
}
}

/// The recorder settings.
public private(set) var settings: [AVMediaType: [String: any Sendable]] = HKStreamRecorder.defaultSettings
/// The recording file name.
public private(set) var fileName: String?
/// The recording output url.
public var outputURL: URL? {
return writer?.outputURL
}
/// The recording or not.
public private(set) var isRecording = false
/// The the movie fragment interval in sec.
public private(set) var movieFragmentInterval: Double?
public private(set) var videoTrackId: UInt8? = UInt8.max
public private(set) var audioTrackId: UInt8? = UInt8.max
#if os(iOS)
public private(set) lazy var moviesDirectory: URL = {
URL(fileURLWithPath: NSSearchPathForDirectoriesInDomains(.documentDirectory, .userDomainMask, true)[0])
}()
#else
public private(set) var moviesDirectory: URL = {
URL(fileURLWithPath: NSSearchPathForDirectoriesInDomains(.moviesDirectory, .userDomainMask, true)[0])
}()
#endif
private var isReadyForStartWriting: Bool {
guard let writer = writer else {
return false
Expand All @@ -82,16 +113,6 @@ public actor HKStreamRecorder {
private var videoPresentationTime: CMTime = .zero
private var dimensions: CMVideoDimensions = .init(width: 0, height: 0)

#if os(iOS)
private lazy var moviesDirectory: URL = {
URL(fileURLWithPath: NSSearchPathForDirectoriesInDomains(.documentDirectory, .userDomainMask, true)[0])
}()
#else
private lazy var moviesDirectory: URL = {
URL(fileURLWithPath: NSSearchPathForDirectoriesInDomains(.moviesDirectory, .userDomainMask, true)[0])
}()
#endif

/// Creates a new recorder.
public init() {
}
Expand All @@ -109,21 +130,44 @@ public actor HKStreamRecorder {
}

/// Starts recording.
public func startRecording(_ fileName: String? = nil, settings: [AVMediaType: [String: any Sendable]] = HKStreamRecorder.defaultSettings) async throws {
/// - Parameters:
/// - file: The file path for recording. If nil is specified, a unique file path will be returned automatically.
/// - settings: Settings for recording.
public func startRecording(_ file: URL? = nil, settings: [AVMediaType: [String: any Sendable]] = HKStreamRecorder.defaultSettings) async throws {
guard !isRecording else {
throw Error.invalidState
}

self.fileName = fileName ?? UUID().uuidString
self.settings = settings
var outputURL: URL
if let file {
Copy link

Choose a reason for hiding this comment

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

If you pass an URL that represents a file without a path extension (e.g. file:///var/mobile/Containers/Data/Application/7910F2B5-9C52-42CE-87D0-20202839EB58/Documents/Demo ) this logic would result in something like this: file:///var/mobile/Containers/Data/Application/7910F2B5-9C52-42CE-87D0-20202839EB58/Documents/file:///var/mobile/Containers/Data/Application/7910F2B5-9C52-42CE-87D0-20202839EB58/Documents/Demo.mp4, which is an incorrect file path.

A more future-proof suggestion to the file parameter and the outputURL handling:

    if let file {
          if file.hasDirectoryPath {
              outputURL = file.appendingPathComponent(UUID().uuidString)
          } else {
              outputURL = file
          }
          if outputURL.pathExtension == "" {
              outputURL = outputURL.appendingPathExtension(Self.defaultPathExtension)
          }
      } else {
          outputURL = moviesDirectory.appendingPathComponent(UUID().uuidString).appendingPathExtension(Self.defaultPathExtension)
      }

Tested with:

  • URL representing a directory.
  • URL representing a file without extension
  • URL representing a file with extension
  • nil URL

Copy link
Owner Author

Choose a reason for hiding this comment

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

Thank you for testing it. I’ve also added some test cases on my end. I’m thinking of adding the documentation and then merging.

if file.hasDirectoryPath {
outputURL = file
} else {
outputURL = moviesDirectory.appendingPathComponent(file.absoluteString)
}
if file.pathExtension == "" {
outputURL = outputURL.appendingPathExtension(Self.defaultPathExtension)
}
} else {
outputURL = moviesDirectory.appendingPathComponent(UUID().uuidString).appendingPathExtension(Self.defaultPathExtension)
}

if FileManager.default.fileExists(atPath: outputURL.absoluteString) {
Copy link

Choose a reason for hiding this comment

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

This should be outputURL.path.

throw Error.fileAlreadyExists(outputURL: outputURL)
}

guard let fileName = self.fileName else { throw Error.invalidState }
var fileType: AVFileType = .mp4
if let supportedFileType = SupportedFileType(rawValue: outputURL.pathExtension) {
fileType = supportedFileType.fileType
} else {
throw Error.notSupportedFileType(pathExtension: outputURL.pathExtension)
}

writer = try AVAssetWriter(outputURL: outputURL, fileType: fileType)
videoPresentationTime = .zero
audioPresentationTime = .zero
self.settings = settings

let url = moviesDirectory.appendingPathComponent(fileName).appendingPathExtension("mp4")
writer = try AVAssetWriter(outputURL: url, fileType: .mp4)
isRecording = true
}

Expand Down