Performance hit for media over 1 hour

[pietrop]

Describe the bug

There seems to be a noticeable performance hit for media over 1 hour - in this case 2h 42 min video.

To Reproduce

Prep - get a transcript for along file

these are the steps I followed, irrelevant for the bug itself, but helpful to setup problem, you can skip

• I got this video from vimeo
• used pietrop/electron-video-downloader to get the video, (electron wrapper to youtubedl)
• created a video over 2 hours, closer to 3 by concating shorter video with ffmpeg (ffmpeg-remix on an mp4)
• create transcript using autoedit.io using pocketsphinx option for STT
• exported autoEdit2 json once transcript is ready

Steps to reproduce the behavior:

1. get a long media and STT json transcript
2. Go to storybook demo page
3. Click on 'import transcript' select 'autoEdit2'
   This is the json I used
   three-hours-sample.mp4_26_04_2019_15-12-51.json.zip
4. Click on 'play'
5. Scroll down to text
6. Click on a word, and start typing
7. See error - there's a noticeable lag between when you type and when you see it on screen

Expected behaviour

No lag for videos over 1 hour, same responsiveness as for video under 1 hour.

Screenshots

Desktop

• OS: Mac OSX 10.13.6 - High Sierra
• Browser: Chrome Version 73.0.3683.103 (Official Build) (64-bit)
• Version: @bbc/[email protected]

Additional context

Not sure what's the issue 🤷‍♂️

Might be mostly to do with draftjs?

facebookarchive/draft-js#437

facebookarchive/draft-js#117

Since we use persistent data and aggressively avoid re-rendering, the size of the document should generally not be a problem unless there is a bottleneck somewhere.

https://reactrocket.com/post/draft-js-best-practices/

A feature-rich draft.js editor with lots of decorators and custom block components can easily cause performance issues, especially for large documents. Draft.js doesn’t do anything special to prevent unnecessary rendering of these components, you can however optimize by using the shouldComponentUpdate method, which puts you in control of when your components re-render.

[Laurian]

I used https://github.com/joshwnj/react-visibility-sensor in a similar project, see https://github.com/bbc/subtitalizer/blob/67b9053c0dc630eaf9a4d33afc75379371dd6066/src/components/WrapperBlock.js#L44

[pietrop]

Thanks @Laurian , thought you had come across this before on that project.

posting code here as we haven't got around open sourcing the other project just yet.

import VisibilitySensor from 'react-visibility-sensor';
...

 return (
      <div className="WrapperBlock">
        <VisibilitySensor intervalCheck={false} scrollCheck>
          {
            ({ isVisible }) =>
              (
                <div>{isVisible ? (
                  <TimecodeBlock
                    start={start}
                    min={min}
                    max={max}
                    end={end}
                    id={id}
                    retime={(time, tcStart, tcEnd) => this.props.blockProps.retime(time, key, tcStart, tcEnd)}
                    focus={focus => this.props.blockProps.focus(focus)}
                  />
                ) : <span>…</span>}
                </div>
              )
          }
        </VisibilitySensor>
        <EditorBlock {...this.props} />
      </div>
    );

so, in subtitalizer I kept draft.js as it was but rendered the timecode widget (you have your speaker widgets instead) only when it was visible

basically not rendering any extra decoration around a draftjs block unless it is visible

ok, ok great thanks! going to try out

[pietrop]

@Laurian I've added it

return (
      <div className={ style.WrapperBlock }>
        <VisibilitySensor intervalCheck={ false } scrollCheck>
          {
            ({ isVisible }) =>
              (
                <div>
                  { isVisible ? (
                    <div className={ [ style.markers, style.unselectable ].join(' ') }
                      contentEditable={ false }>
                      {this.props.blockProps.showSpeakers ? speakerElement : ''}

                      {this.props.blockProps.showTimecodes ? timecodeElement : ''}
                    </div>
                  ) : <span>…</span> }
                </div>
              )
          }
        </VisibilitySensor>
        <div className={ style.text }>
          <EditorBlock { ...this.props } />
        </div>
      </div>
    );

made a branch performance-issue-on-long-files

performance is the same, and also the VisibilitySensor component does not seem to be working?

I remember there was an issue around, the WrapperBlock component not updating.
Eg if you go in settings, ⚙️ and change timecode offset, to get that showing in WrapperBlock through TimedTextEditor we had to force a re-render for draftJs 🤷‍♂ - perhaps that's related? and it's not setup correctly?

[jkokatnur]

Is there any update on this issue, we are using this module in our application and also facing a noticeable lag between typed text and reflecting it on the screen. This behaviour is mostly for the large size audio files.

[pietrop]

Hi @jkokatnur
We have a PR where we have been doing some optimisation reducing un-necessary re-renders
#160 however it's still pending review.

And see the last comment in the PR about "paginating" draftJs content, as is something that might be worth exploring as separate branch/PR #160 (comment)

Btw, did you notice what's the duration of audio after which you start to get performance issues?

Do Let us know if you have any thoughts or suggestions on how to tackle this.

[pietrop]

Test update

despite this issue Automattic/simplenote-electron#501 (comment) I've tried with text that has 22 763 words corrsponding to 2h42min.txt copy pasting in draftjs.org demo (so without entities) and it seems like it is editable just fine..

However if you try and copy paste the whole of the Moby Dick book - 215 821 words then it struggles.

[Laurian]

This kinda works, needs more testing but it feels the right direction: Laurian@63924f2#diff-cdf6a9957d22ff2efacaaf5fb2876a8aL111-L114

Basically if a block is off-screen render a read-only lighter DOM version of it (just text no entities)

[pietrop]

Thanks @Laurian !

Did you test yours by trying to correct the text in the 3 hour long programme? and if so was there any lag during the typing?

We used to have something similar, but then I removed it in this PR #181 or are your changes different?

[Laurian]

hmm, yeah the treatment of the EditorBlock is as in PR #181, wasn't that working for you?

[pietrop]

Wasn't working, when correcting text there was still a significant lag. But at this point not 100% sure what exactly is causing it?

[NickRuiz]

I'm also having this performance issue on a 24M wav file (approx 25 minutes). The rendered draftJs file is 2.4M. I've noticed Google Chrome spiking at 90% while playing, causing various actions to take place in different orders from my user actions. I think all of the issues reported today are due to the latency problem.

[pietrop]

via @Laurian https://www.chromestatus.com/feature/4613920211861504

Adds the rendersubtree attribute to all HTML elements, which locks a DOM element for display. When rendersubtree is set to "invisible", the element's content is not drawn or hit-tested, allowing for rendering optimizations. The rendersubtree "activatable" token allows the browser to remove the invisible attribute, rendering the content, and making it visible.

this combined with visibility-sensor could be used to speed up long transcripts
I will test it later

[aneesijaz]

Also having same problem. video is over an hour and sometimes even page crashes due to heavy load.

[aneesijaz]

ok this worked for me.. no lag at all.
wrapperBlock.js:

     let containmentDOMRect = document.querySelector(".notranslate");
     return (
      containmentDOMRect ? (
        <VisibilitySensor partialVisibility={true} onChange={this.visibilityChanged} containment={containmentDOMRect} intervalCheck={ false } scrollCheck>
         {({isVisible}) =>  isVisible ?
          <div className={ style.WrapperBlock }>
              <div
                className={ [ style.markers, style.unselectable ].join(' ') }
                contentEditable={ false }
              >
                {this.props.blockProps.showSpeakers ? speakerElement : ''}

                {this.props.blockProps.showTimecodes ? timecodeElement : ''}
              </div>
              <div className={ style.text }>
                <EditorBlock { ...this.props } />
              </div>
            </div> : <span className={ style.loadingBlockPlaceholder }>loading...</span>
          }
        </VisibilitySensor>
      ):null
    );

[pietrop]

That's awesome thanks @aneesijaz !! will try it out as soon as I get a chance!
PR also welcome ;)

[pietrop]

re

let containmentDOMRect = document.querySelector(".notranslate");

@aneesijaz where do you get the .notranslate element from?

[aneesijaz]

That's the css selector for the div holding all the wrapperBlocks (may be generated by draft.js).
I think the issue was by default visibility sensor was not working because it wasn't picking the containment automatically so I just added it.
Thanks.

[pietrop]

Stating the obvious, if you indulge me for a sec, re content editable vs textarea.

Took one hour worth of video, copy pasted twice, so that would be equivalent to two hours worth of text transcription.

Put a text area along side a content editable div. (did this in this other app coz I was already using a play content editable area there - without draftJs)

As you can see editing text in text area is snappy, even deleting, while there is substantial lag in content editable (on the right).

I know that DraftJs constrains content editable to make it more efficient. But none the less is still build on top of it under the hood, so there might be some limitations that are to do with the dom, the browser, and content editable behaviour when it's content is too large, requiring repainting etc..

Other thought is that we might not need the rich text editing experience traditional of content editable and draftJS and if we park the highlighting words while playing feature for a second. Exploring textarea might lead to some performance boost.

So the idea is, could we do a TimedTexteditor that instead of DraftJS uses textarea, and how far would we go in replicating the existing feature set?

I guess another issue would be how to display paragraphs/speakers?
I had a got at this before this project, in a previous project pietrop/fact2_transcription_editor altho I was using content editable and not a textarea there.

---

Example JsFiddle

• Double clicking on a word (returns start char and end char of that word) play corresponding point in video/audio.
• single click gets the char offset position
• how to represent speakers at paragraph level in a textarea(?) eg marked in between ~(?) on its own line (?)
• Words highlighted at current time nice to have
• Scroll Sync, keep current word in view <— (toggle on/off) nice to have
• Preserve timecodes while editing - TBC how
  This last one, might not be necessary, it could just be that if you click on the words you get roughly to that point, and not necessarily the to the exact word. And that the realignment of timecodes is done on save when done editing. Or at a less frequent interval etc..

[pietrop]

Following @Laurian 's advised and before trashing draftJS had a go at removing entities from DraftJs to simplify the setup. see this PR for more details pietrop#13
btw made the PR agains my fork, but can make one agains the BBC Repo if it proves to work.

[dash7ou]

isEditable={true} but i can not edit text in browser?!