HiveBrain v1.2.0
Get Started
← Back to all entries
patterncsharpMinor

WPF Multiscreen Media Application

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
wpfmultiscreenmediaapplication

Problem

Information

I have a Multiscreen Application written for a Client that basically is displaying Video content with MediaElements bind to a ContentControl on 5 Screens horizontally(5 splittet Videos). Depending on wich view the User is im playing different 5 Videos and Stopping the Other Videos.

I've put the MediaElements in a library and at App Start I'm loading the 5x5 Videos with 1920x1080(mp4).
The App runs ok but I'm not happy with my Code, and I have the feeling that this could be done much better and I always keep questioning myself because of my Code.

Since I'm working alone and I don't know other Devs I would love to hear criticism about my Code because I want to learn to get better

The Code

```
public class VideoWallViewModel : BindableBase {
public VideoWallViewModel() {
Initialize();
}

#region Functions

private void Initialize() {
_iEventAggregator = Event.EventInstance.EventAggregator;

MediaElement0 = new MediaElement();
MediaElement1 = new MediaElement();
MediaElement2 = new MediaElement();
MediaElement3 = new MediaElement();
MediaElement4 = new MediaElement();

MediaElement0.BeginInit();
MediaElement1.BeginInit();
MediaElement2.BeginInit();
MediaElement3.BeginInit();
MediaElement4.BeginInit();
MediaElement0.LoadedBehavior = MediaState.Manual;
MediaElement1.LoadedBehavior = MediaState.Manual;
MediaElement2.LoadedBehavior = MediaState.Manual;
MediaElement3.LoadedBehavior = MediaState.Manual;
MediaElement4.LoadedBehavior = MediaState.Manual;

MediaElement0.Loaded += MediaElementOnLoaded;
MediaElement1.Loaded += MediaElementOnLoaded;
MediaElement2.Loaded += MediaElementOnLoaded;
MediaElement3.Loaded += MediaElementOnLoaded;
MediaElement4.Loaded += MediaElem

Solution

A way to reduce the amount of code in initialize and allow it to be more flexible is by putting your MediaElements into an array:

var mediaElems = new[]{
    new MediaElement(),
    new MediaElement(),
    new MediaElement(),
    new MediaElement(),
    new MediaElement()
}

foreach(var elem in mediaElems){
    elem.BeginInit();
    elem.LoadedBehavior = MediaState.Manual;
    elem.Loaded += MediaElementOnLoaded;
    elem.MediaOpened += MediaElementOnMediaOpened;
    elem.MediaEnded += MediaElementOnMediaEnded;
    elem.MediaFailed += MediaElement0OnMediaFailed;
    elem.EndInit();
    elem.Volume = 0;
}


If you let this be a field of your class this._mediaElems you can also reduce Pause, Play and Stop

private void Pause() {
    foreach(var elem in _mediaElems){
        elem.Pause();
    }
}

private void Play() {
    foreach(var elem in _mediaElems){
        elem.Play();
    }
}

private void Stop() {
    foreach(var elem in _mediaElems){
        elem.Stop();
    }
}


You defined ApplyState but you forgot to use it on MediaChanged.
You should apply the same array idea to VideoWallMedia, however I would first start by looking at VideoWallMedia and MediaElement and see how they are related, it seems they are not modeled correctly.

This method is not being referenced anywhere, maybe you can delete the code?

private void MediaChanged(VideoWallMedia media) {
    _mediaElems[0].Source = media.Video0;
    _mediaElems[1].Source = media.Video1;
    _mediaElems[2].Source = media.Video2;
    _mediaElems[3].Source = media.Video3;
    _mediaElems[4].Source = media.Video4;

    ApplyState();
}

Code Snippets

var mediaElems = new[]{
    new MediaElement(),
    new MediaElement(),
    new MediaElement(),
    new MediaElement(),
    new MediaElement()
}

foreach(var elem in mediaElems){
    elem.BeginInit();
    elem.LoadedBehavior = MediaState.Manual;
    elem.Loaded += MediaElementOnLoaded;
    elem.MediaOpened += MediaElementOnMediaOpened;
    elem.MediaEnded += MediaElementOnMediaEnded;
    elem.MediaFailed += MediaElement0OnMediaFailed;
    elem.EndInit();
    elem.Volume = 0;
}
private void Pause() {
    foreach(var elem in _mediaElems){
        elem.Pause();
    }
}

private void Play() {
    foreach(var elem in _mediaElems){
        elem.Play();
    }
}

private void Stop() {
    foreach(var elem in _mediaElems){
        elem.Stop();
    }
}
private void MediaChanged(VideoWallMedia media) {
    _mediaElems[0].Source = media.Video0;
    _mediaElems[1].Source = media.Video1;
    _mediaElems[2].Source = media.Video2;
    _mediaElems[3].Source = media.Video3;
    _mediaElems[4].Source = media.Video4;

    ApplyState();
}

Context

StackExchange Code Review Q#156742, answer score: 7

Revisions (0)

No revisions yet.