samirkumardas / jmuxer

jMuxer - a simple javascript mp4 muxer that works in both browser and node environment.
Other
548 stars 108 forks source link

Add video and audio error handlers #121

Closed stevenwebdev closed 1 year ago

stevenwebdev commented 1 year ago

This PR allows a user to catch any video/audio feeding errors and act accordingly (show UI error states, make API request etc).

stevenwebdev commented 1 year ago

We could also just use the existing onError handler:

if (typeof this.options.onError === 'function') {
    this.options.onError.call(null, data);
}

instead of creating new ones, but I didn't want to change its primary purpose.

samirkumardas commented 1 year ago

Thank you for your afford and time. Adding an error callback here won't help much. For example, it might have some chunks with empty frames during ongoing streaming, but usually, it does not impact playing as jMuxer skips those empty chunks. These error callbacks won't help unless we get all chunks with zero frames i.e. invalid media data. In such a case, it has an onError callback.

stevenwebdev commented 1 year ago

Thank you for your afford and time. Adding an error callback here won't help much. For example, it might have some chunks with empty frames during ongoing streaming, but usually, it does not impact playing as jMuxer skips those empty chunks. These error callbacks won't help unless we get all chunks with zero frames i.e. invalid media data. In such a case, it has an onError callback.

I'm working right now on a project where we are playing raw H264 (not stream). We get videos from ROS message that we parse into array bytes and then feed into JsMuxer. So these would definitely help us to cover the feeding error in case of a corrupted ROS bag, but I understand for streaming it has no use.

samirkumardas commented 1 year ago

@stevensturkop Thanks for the detail. I have a question though

When you get a ROS message with an empty frame, what action do you take using those callbacks?

stevenwebdev commented 1 year ago

@stevensturkop Thanks for the detail. I have a question though

When you get a ROS message with an empty frame, what action do you take using those callbacks?

If the ROS message parsing somehow got corrupted, an example could be that the first N bytes corresponding to the headers were not removed/wrongly encoded, in that case we would still have the H264 raw packet but it wouldn't be playable.

Since we are rendering a playback video and not a stream, we want to dispatch a redux action showing an error overlay on top of the video placeholder with something like "Sorry there has been a problem rendering this video".

samirkumardas commented 1 year ago

Sorry, I was busy last week. I got your point. As I mentioned it would fire every time when it found a chunk without frames in case of streaming. In that case, it is not an error, it is a common scenario in streaming.

Could you please rename the callback name onVideoFeedError to onMissingAudioFrames and onAudioFeedError to onMissingAudioFrames? Then we are good to merge.

stevenwebdev commented 1 year ago

Sorry, I was busy last week. I got your point. As I mentioned it would fire every time when it found a chunk without frames in case of streaming. In that case, it is not an error, it is a common scenario in streaming.

Could you please rename the callback name onVideoFeedError to onMissingAudioFrames and onAudioFeedError to onMissingAudioFrames? Then we are good to merge.

No worries, I understand.

Sweet, will do by EOD today :)

stevenwebdev commented 1 year ago

@samirkumardas Could you also publish the changes to NPM, whenever you have time 🙂

Thank you

samirkumardas commented 1 year ago

Thank you @stevensturkop

Released a version