Update timestamps using diff alignment algorithm #144

murezzda · 2019-04-12T06:50:01Z

Is your Pull Request request related to another issue in this repository ?
Related to #30

Describe what the PR does
To restore timestamps after editing of the transcript, the new state is compared to the original state of the transcript. diff-match-patch is used to find all differences between the original and new transcript, and the following rules are applied:

If a new word matches the original word, move the timestamps to the new word
If a word is removed, ignore the original timestamp.
If a word is added, set the timestamp to the previous original word (in this case, two consecutive words will have the same timestamp).
If multiple words are replaced by a single word, set the timestamp of that word to the combined timestamp of all replaced words.
If multiple words are replaced by multiple words, set all timestamps of the new words to the combined timestamp of all replaced words.

State whether the PR is ready for review or whether it needs extra work
The following points need to be clarified

The point of updating the timestamps is currently a timer (like the save document timer). The timestamp update can take some time, especially on larger files, thus it might be better to trigger this by hand.
Saving the "original" transcript might need some adjustments.

pietrop · 2019-04-12T08:50:02Z

Thanks @murezzda , very interesting approach!

There's a module, we just open sourced stt-align-node, latest changes are on diffs-report branch in diffs-report/lib/index.js which does a similar diffing, with a different diffing library. (Originally developed by @chrisbaume in BBC R&D in python) - apologies for not open sourcing it sooner.

See more details in this comment in this issue #30 on how I've tried to use it with the TranscriptEditor.

In this case that module is able to do an interpolation of the new inserted words, and transpose the time-codes for the matching once. As you can see explained in the README

I think you are onto something, and there might be a way to combine elements of these two things for an optimal approach.

Laurian · 2019-04-12T09:53:21Z

Ideally we could devise later (as in: accept this PR and refactor later) a mechanism to swap implementations of various timestamp correcting things, like a plugin style or something. Such that we could have: this, stt-align-node, remote API call (Gentle, Aeneas, Speechmatics aligner)

murezzda · 2019-04-12T11:18:57Z

Thanks for the input! I think for the next step I will check out stt-align-node. If it seems feasible, I will integrate it in my code.

murezzda · 2019-04-15T12:36:24Z

I've added now the stt-align-node code and use it as the current method to update the timestamps. The diff-match-patch method is still in the PR, but currently not used.

What I've experienced so far with stt-align-node is that it works generally better for insertion and mutliple words substitiutions, but at the beginning of a longer pause words can have timestamps far to long into the pause instead at the beginning of the pause. This might be improved by combining stt-align-node with diff-match-patch, which I want to try next.

pietrop · 2019-04-15T16:20:42Z

Thanks @murezzda !

See if this can help with the testing, on the Ted website, there is the "human accurate" transcript for this ted Talk.

Here for convenience, with line spaces, and time-codes removed

There was a day, about 10 years ago, when I asked a friend to hold a baby dinosaur robot upside down. It was this toy called a Pleo that I had ordered, and I was really excited about it because I've always loved robots. And this one has really cool technical features. It had motors and touch sensors and it had an infrared camera. And one of the things it had was a tilt sensor, so it knew what direction it was facing. And when you held it upside down, it would start to cry. And I thought this was super cool, so I was showing it off to my friend, and I said, "Oh, hold it up by the tail. See what it does." So we're watching the theatrics of this robot struggle and cry out. And after a few seconds, it starts to bother me a little, and I said, "OK, that's enough now. Let's put him back down." And then I pet the robot to make it stop crying.
And that was kind of a weird experience for me. For one thing, I wasn't the most maternal person at the time. Although since then I've become a mother, nine months ago, and I've learned that babies also squirm when you hold them upside down.
But my response to this robot was also interesting because I knew exactly how this machine worked, and yet I still felt compelled to be kind to it. And that observation sparked a curiosity that I've spent the past decade pursuing. Why did I comfort this robot? And one of the things I discovered was that my treatment of this machine was more than just an awkward moment in my living room, that in a world where we're increasingly integrating robots into our lives, an instinct like that might actually have consequences, because the first thing that I discovered is that it's not just me.
In 2007, the Washington Post reported that the United States military was testing this robot that defused land mines. And the way it worked was it was shaped like a stick insect and it would walk around a minefield on its legs, and every time it stepped on a mine, one of the legs would blow up, and it would continue on the other legs to blow up more mines. And the colonel who was in charge of this testing exercise ends up calling it off, because, he says, it's too inhumane to watch this damaged robot drag itself along the minefield. Now, what would cause a hardened military officer and someone like myself to have this response to robots?
Well, of course, we're primed by science fiction and pop culture to really want to personify these things, but it goes a little bit deeper than that. It turns out that we're biologically hardwired to project intent and life onto any movement in our physical space that seems autonomous to us. So people will treat all sorts of robots like they're alive. These bomb-disposal units get names. They get medals of honor. They've had funerals for them with gun salutes. And research shows that we do this even with very simple household robots, like the Roomba vacuum cleaner.
It's just a disc that roams around your floor to clean it, but just the fact it's moving around on its own will cause people to name the Roomba and feel bad for the Roomba when it gets stuck under the couch.
And we can design robots specifically to evoke this response, using eyes and faces or movements that people automatically, subconsciously associate with states of mind. And there's an entire body of research called human-robot interaction that really shows how well this works. So for example, researchers at Stanford University found out that it makes people really uncomfortable when you ask them to touch a robot's private parts.
So from this, but from many other studies, we know, we know that people respond to the cues given to them by these lifelike machines, even if they know that they're not real.
Now, we're headed towards a world where robots are everywhere. Robotic technology is moving out from behind factory walls. It's entering workplaces, households. And as these machines that can sense and make autonomous decisions and learn enter into these shared spaces, I think that maybe the best analogy we have for this is our relationship with animals. Thousands of years ago, we started to domesticate animals, and we trained them for work and weaponry and companionship. And throughout history, we've treated some animals like tools or like products, and other animals, we've treated with kindness and we've given a place in society as our companions. I think it's plausible we might start to integrate robots in similar ways.
And sure, animals are alive. Robots are not. And I can tell you, from working with roboticists, that we're pretty far away from developing robots that can feel anything. But we feel for them, and that matters, because if we're trying to integrate robots into these shared spaces, we need to understand that people will treat them differently than other devices, and that in some cases, for example, the case of a soldier who becomes emotionally attached to the robot that they work with, that can be anything from inefficient to dangerous. But in other cases, it can actually be useful to foster this emotional connection to robots. We're already seeing some great use cases, for example, robots working with autistic children to engage them in ways that we haven't seen previously, or robots working with teachers to engage kids in learning with new results. And it's not just for kids. Early studies show that robots can help doctors and patients in health care settings.
This is the PARO baby seal robot. It's used in nursing homes and with dementia patients. It's been around for a while. And I remember, years ago, being at a party and telling someone about this robot, and her response was, "Oh my gosh. That's horrible. I can't believe we're giving people robots instead of human care." And this is a really common response, and I think it's absolutely correct, because that would be terrible. But in this case, it's not what this robot replaces. What this robot replaces is animal therapy in contexts where we can't use real animals but we can use robots, because people will consistently treat them more like an animal than a device.
Acknowledging this emotional connection to robots can also help us anticipate challenges as these devices move into more intimate areas of people's lives. For example, is it OK if your child's teddy bear robot records private conversations? Is it OK if your sex robot has compelling in-app purchases?
Because robots plus capitalism equals questions around consumer protection and privacy.
And those aren't the only reasons that our behavior around these machines could matter. A few years after that first initial experience I had with this baby dinosaur robot, I did a workshop with my friend Hannes Gassert. And we took five of these baby dinosaur robots and we gave them to five teams of people. And we had them name them and play with them and interact with them for about an hour. And then we unveiled a hammer and a hatchet and we told them to torture and kill the robots.
And this turned out to be a little more dramatic than we expected it to be, because none of the participants would even so much as strike these baby dinosaur robots, so we had to improvise a little, and at some point, we said, "OK, you can save your team's robot if you destroy another team's robot."
And even that didn't work. They couldn't do it. So finally, we said, "We're going to destroy all of the robots unless someone takes a hatchet to one of them." And this guy stood up, and he took the hatchet, and the whole room winced as he brought the hatchet down on the robot's neck, and there was this half-joking, half-serious moment of silence in the room for this fallen robot.
So that was a really interesting experience. Now, it wasn't a controlled study, obviously, but it did lead to some later research that I did at MIT with Palash Nandy and Cynthia Breazeal, where we had people come into the lab and smash these HEXBUGs that move around in a really lifelike way, like insects. So instead of choosing something cute that people are drawn to, we chose something more basic, and what we found was that high-empathy people would hesitate more to hit the HEXBUGS.
Now this is just a little study, but it's part of a larger body of research that is starting to indicate that there may be a connection between people's tendencies for empathy and their behavior around robots. But my question for the coming era of human-robot interaction is not: "Do we empathize with robots?" It's: "Can robots change people's empathy?" Is there reason to, for example, prevent your child from kicking a robotic dog, not just out of respect for property, but because the child might be more likely to kick a real dog?
And again, it's not just kids. This is the violent video games question, but it's on a completely new level because of this visceral physicality that we respond more intensely to than to images on a screen. When we behave violently towards robots, specifically robots that are designed to mimic life, is that a healthy outlet for violent behavior or is that training our cruelty muscles? We don't know ... But the answer to this question has the potential to impact human behavior, it has the potential to impact social norms, it has the potential to inspire rules around what we can and can't do with certain robots, similar to our animal cruelty laws. Because even if robots can't feel, our behavior towards them might matter for us. And regardless of whether we end up changing our rules, robots might be able to help us come to a new understanding of ourselves.
Most of what I've learned over the past 10 years has not been about technology at all. It's been about human psychology and empathy and how we relate to others. Because when a child is kind to a Roomba, when a soldier tries to save a robot on the battlefield, or when a group of people refuses to harm a robotic baby dinosaur, those robots aren't just motors and gears and algorithms. They're reflections of our own humanity.
Thank you.

I've also noticed that the colour highlight in TimedTextEditor while playing is not a great measure to check the accuracy of the alignment, as it might drag behind a little bit, best bet, is to double click on a word, and see if corresponding sound comes out.

Other than that, seems pretty accurate!

murezzda · 2019-04-29T15:00:10Z

Update for this branch:

I've tried to merge the two attempts, but as I discovered they are not as easily compatible as I though. For the moment, I wont persuade this method anymore.
Updating the timestamps causes the focus to be lost. This is not that big of a problem at long intervals between timestamp updating, but on shorter intervals it is no longer practical to edit the text.

pietrop · 2019-04-29T16:08:16Z

sounds good, re not merging the two diffing approaches.

As for point 2, it could either be done on a "final save" (eg just before sending to the server) or more granularly at paragraph level, eg once user moves onto second paragraph, and see if this doesn't loose the focus. Or the focus could be restored after the update.

I also think there might be tweaks to address this issue #150 that will effect this PR.

Thanks @murezzda will have a closer look as soon as I get a chance.

… instead of doing a 2-step process.

…sst-align code

…ords. Timestamp update function now also uses the alignWords function directly instead of alignJSONText, removing some overhead.

murezzda · 2019-06-05T13:32:46Z

Selection is now preserved when the timestamp is updated, which improves the usability significantly. The update of the selection state is done by mapping the old selection state on the new state. Performance wise the impact of moving the selection state seems neglible.

murezzda · 2019-06-05T13:40:21Z

Regarding the overall performance of the update, it still introduces a lag if the file is long (such as the Kate Darling TED Talk).

I took some measurements and it seems the alignment of the timestamps is not the main factor, the conversion and updating of the state also takes at least as much time as the alignment:

Conversion of state to Raw: 33.01611328125ms
STT alignment: 47.970947265625ms
Conversion of state from Raw: 58.0439453125ms
Update editor state: 49.7158203125ms

As previously mentioned, updating the selection takes about 42.7 ms in total and thus has about the same impact on the performance as all other functions used here.

…uring timestamp update

pietrop · 2019-06-05T15:15:49Z

Thanks for this @murezzda
Very interesting the measurements, I wonder if there' s a way to narrow down the update to only the paragraph where the text has changed? and whether that would make any difference?

murezzda · 2019-06-06T12:52:52Z

Hi @pietrop

The problem is that the conversion to and from raw anyway have to happen, thus it would only be possible to save time in the STT alignment section. I've also thought about checking if something changed in the transcript and only align if that is the case, but it looks to me that obtaining the information where and if something has changed is more or less as expensive as just aligning the text itself.

Whats your opinion, should we try to improve the performance or could we also handle this by placing the call to the realignment at some different point in the application? At the moment, the realignment happens whenever the current state of the transkript is saved.

Regarding the measurements, I've used the console.time('test') and console.timeEnd('test') methods. Do you know if these are reliable for react?

pietrop · 2019-06-06T15:05:47Z

Hi @murezzda,
I think placing it on save could be a good choice.

Altho perhaps instead of the automatic save( that happens when people stop typing), we could place it after the manual save btn?

And add a way so that when a parent component exports the content, a realignment is done if text has been changed since the last one - if that makes sense.

Re measurements, other might be able to advise? (@Laurian @jamesdools @barneyboo )

In this PR #160 where we looked at the unnecessary re-renders I made some notes and one of the things I found was the React DevTools tools profiler - see in react docs but I am not sure if/how to use that to measure that type of performance? but might be worth looking into it.

murezzda · 2019-06-07T13:36:52Z

Hi @pietrop

I've now tried to put the update as a timer into the if (this.state.editorState.getCurrentContent() !== editorState.getCurrentContent()) code of the onChange function. This improves the user-experience, as alignments only happen if something has changed in the text.

Whats a bit of a problem with this is that it can be that unaliged text can be automatically saved, but as the alignment anyways happens on the "original" transcript, this should not be that big of a problem. Especially if the alignment is also triggered by the manual save button, this gives the user a way to trigger the aligment explicitly.

Let me know what you think of this. If this would be a viable solution, I would clean up the code and give it up for review.

pietrop · 2019-06-07T15:33:19Z

@murezzda this sounds great!
Feel free to clean it up and let me know when is ready for review, I can make some time next week to go through it 😄

…ter 5 seconds if the transcript has been edited or if the user saves the transcript manually with the save button

murezzda · 2019-06-11T18:41:13Z

@pietrop cleanup done, ready for review.

pietrop

Thanks a lot for this @murezzda!! it's looking good

Testing - Ted talk

I found the "highlighted word" lagged behind a little bit, so reading along while playing wasn’t necessary a good measure for accuracy of the realignment. So was most efficient to test by double clicking on a word and checking if it plays from the corresponding sound.

Other then that, in my testing with the Ted Talk, it seemed like a pretty accurate realignment (expect for the last word of most paragraph, and at the end of the transcript).

Timings in last word in paragraphs

While doing the test described above, it seems like the timings of the the second last word include the timings of the last word. So if you double click on the last word of paragraph, you don’t get the word but the pause before the next paragraph is spoken.
There might be possible tweaks on the underlying algorithm to improve this, if is a systematic thing.(seems to happen in the majority of paragraphs but not 100% all of them) 

this could be addressed in a subsequent PR

Difference between pasting and typing

if you paste text, there might not be entities for those words, and somehow the data attribute is empty,
if you type and insert some words the speaker name at paragraph level doesn’t get replaced.

To test using the ted talk, In the demo app, I have deleted all of the text, and pasted the accurate transcript from the Ted Talk.

I seem to get NaN:NaN:NaN: for time-codes and no speaker names.

I think there might be two extra steps needed

one is adding start timecode attribute to the draftJS blocks after realignment
two is seeing if we can retain the speak names

Made a separate PR with some more info and possible workarounds murezzda#1

this could be addressed in a subsequent PR

Questions

inside updateTimestamps would it make a difference if we were to align at paragraph/block level?

Other thoughts

Just to recap timestampTimer is setup similar to saveTimer and it gets triggered 5 seconds after the user stops typing. And updateTimestampsForEditorState can also be triggered by the save btn.

Possible separate PRs

could also consider (as separate PR) adding a rsync btn eg 🔄with on hover, something about "restrore timecodes for newly corrected words" - or something similar? if making the resync more explict, altho it might be confusing for users? so might want to just "hide it behind the save".
As a separate PR, we could add a way to make the restoring timecodes an option from the outer component when retrieving data.
So that when saving to server there’s more certainty on whether there are time-codes for each word.

As mentioned, would be ideal to merge this PR #160 into master first, around reducing unnecessary re-renders but we are a bit stretch in terms of finding someone to review it on our end before merging it in, let me know if you got time to have a look and leave some comments (if needed) it be interesting to hear your thoughts on some of those choices.

pietrop · 2019-06-23T10:27:31Z

package.json

@@ -46,8 +46,12 @@
    "@fortawesome/free-solid-svg-icons": "^5.6.3",
    "@fortawesome/react-fontawesome": "^0.1.3",
    "babel-polyfill": "^6.26.0",
+    "diff-match-patch": "^1.0.4",


Suggested change

"diff-match-patch": "^1.0.4",

diff-match-patch doesn’t seem to be in use in package.json could be removed?

Yes, this is from the previous attempt to fix the timecodes via the diff-match-patch lib. Thanks for finding this.

pietrop · 2019-06-23T10:30:55Z

packages/components/timed-text-editor/UpdateTimestamps/index.js

+import alignWords from './stt-align-node.js';
+
+const convertContentToText = (content) => {
+  var text = [];


Suggested change

var text = [];

let text = [];

could replace var with let
some background here

pietrop · 2019-06-23T11:20:37Z

packages/components/timed-text-editor/UpdateTimestamps/index.js

+  const result = alignWords( entities, currentText);
+
+  const newEntities = result.map((entry) => {
+    return createEntity(entry.start, entry.end, 0.0, entry.word, -1);


In createEntity the index is always -1?

Would this be a fix?

const newEntities = result.map((entry, index) => { return createEntity(entry.start, entry.end, 0.0, entry.word, index);

Yes, this would be a fix. At the moment the index is not set in any other way.

pietrop · 2019-06-23T13:15:06Z

Easiest fix for being more certain when retrieving data from the outer component that there are word level timings for each word is to make the restoring time-codes part of the export process.

Motivation as mentioned above is so that when saving to server there’s more certainty on whether there are time-codes for each word.

Possible solution is to add a step in getEditorContent to updateTimestampsForEditorState.

  getEditorContent(exportFormat) {
    const format = exportFormat || 'draftjs';
++ this.updateTimestampsForEditorState();
    return exportAdapter(convertToRaw(this.state.editorState.getCurrentContent()), format);
  }

If done this way, it could be part of this PR.

Just for background context, the current way of getting the editor's content from a parent component can be seen in demo app (demo/app.js) where the exportTranscript function is uses refs and getEditorContent function.

const { data, ext } = this.transcriptEditorRef.current.getEditorContent(
      this.state.exportFormat
    );

pietrop · 2019-07-13T16:46:37Z

murezzda/react-transcript-editor@update-timestamps-diff...bbc:murezzda-update-timestamps-diff

pietrop · 2019-07-16T11:46:52Z

I've run a test with this interview From PBS Frontline Documentary The Facebook Dilemma, Soleio Cuervo, Former Facebook Product Designer

steps to reproduce

Accurate text of transcription and video from youtube via electron-video-downloader

The Facebook Dilemma - Soleio Cuervo-OIAUfZBd_7w.en.txt

Run the transcript via autoEdit using BBC Kaldi (can use any other STT provider tho) and exported as json

The_Facebook Dilemma - Soleio Cuervo-OIAUfZBd_7w.mp4_12_07_2019_15-27-47-autoEdit2-from-Octo-Kaldi.json.txt (remove the .txt to turn it back into a json)

Imported in react-transcript-editor - local version for this branch murezzda-update-timestamps-diff based on this PR Update timestamps using diff alignment algorithm #144 - http://localhost:6006 storybook demo page. (choose autoEdit2 under import transcript options.)
Imported the media downloaded form youtube.

results

The alignment was very good, all the way to the end. 🎉

Caveats

With this algorithm the alignment is only as good as the accuracy of the STT.
The accuracy of STT recognition is effected by

quality of the STT engine
quality of the recording, eg microphone used, background noise, distance of speaker from mic etc..
(presumably) enunciation of words and accents (?)

To try this first hand, used pocketsphinx STT (free, open source and works offline) option in autoEdit to transcribe the same recording.

The_Facebook Dilemma - Soleio Cuervo-OIAUfZBd_7w.mp4_12_07_2019_15-27-47-autoEdit2-from-pocketsphinx.json.txt (remove .txt to convert back to json)

It starts good, and then It drifts out of sync, incrementally. It is out of sync by a paragraph or more by the end of the interview.

For this edge case, where the STT or the recognition might be a bit off, we might not have enough insights on why it's going out of sync, is it just the diffing that doesn't work? is there something that can be added to the algo to handle that etc..?
Or another thought is could we recognise smaller chunks at a time? altho not sure how we'd segment for that?

To note is that for now this might be a relatively small edge case, because with decent STT and decent recordings the problem might be mitigated.

* moved header into a component with shouldComponentUopdate false to avoid unecessary re-renders - test * moved Header component into separate file +removed unused styling, commented out for now in case it's needed, eg in mobile view? * moved playback_rates const into separate file * optimised re-render of playback rate * optimise re-render for VideoPlayer * added some notes - draft on how to prevent uncessary re-renders in React * Added some comments * small note in docs about using console.log in render to measure performance * ammend to notes * Update 2019-05-16-prevent-unnecessary-re-renders-in-react.md * notes update * notes fix * trying out why-did-you-update * updated MediaPlayer and subcomponents * made ToolTip 'how does this work' into it's own component * updated Demo app to reduce unecessary re-renders * added react-visibility-sensor * refactor settings * removed unecessary attributes from state of components + WrapperBlock performance tweak using react-visibility-sensor

jamesdools

Thanks a lot, this is really really cool! 🙌 really appreciate your time on this.

I think most of my comments are about readability of the code - it's quite a lot to digest. I know this has been sitting here for a long time so I don't wish to block this for too much longer while we have Pietro's time.

I approve but would appreciate if a little bit of time went into refactoring - mostly around variable and function naming! A few are quite verbose whilst also not being very descriptive - so could cut them down - usually in the function scope it's quite clear. On the other hand, some are a little too generic. A hard balance, it's something we need to tackle elsewhere in the repo too 😄

But nitpicking aside this is very awesome 👍

jamesdools · 2019-08-12T14:44:08Z

packages/components/timed-text-editor/UpdateTimestamps/stt-align-node.js

+  // http://borischumichev.github.io/everpolate/#linear
+  const outStartTimes = everpolate.linear(indicies, indiciesWithStart, startTimes);
+  const outEndTimes = everpolate.linear(indicies, indiciesWithEnd, endTimes);
+  words = words.map((word, index) => {


probably better to assign this to a new variable in memory as map returns a new array.

jamesdools · 2019-08-12T14:49:51Z

packages/components/timed-text-editor/UpdateTimestamps/stt-align-node.js

+  const startTimes = [];
+  const endTimes = [];
+  // interpolate times for start
+  for (let i = 0; i < words.length; i++) {


Not trying to be a stickler here! But perhaps an ES6 approach might help clear up the code a bit.

eg:

words.forEach((word, index) => { if ('start' in word) { indiciesWithStart.push(index); startTimes.push(word.start); } if ('end' in word) { indiciesWithEnd.push(index); endTimes.push(word.end); } });

(written in the PR box so would test it first) 😛

jamesdools · 2019-08-12T14:58:58Z

packages/components/timed-text-editor/index.js

  loadData() {
    if (this.props.transcriptData !== null) {
      const blocks = sttJsonAdapter(this.props.transcriptData, this.props.sttJsonType);
+      this.setState({ originalState: convertToRaw(convertFromRaw(blocks)) });


I'm missing something here I think - can we rename this better? Makes it sound like it's a deep copy of the state object, but it's the transcript text right. Should we also declare it in constructor?

we need to check with @murezzda what was the thinking around this, I am not quiet sure.

this feels like it was needed as a deep copy? but can't remember why?

this.setState({ originalState: convertToRaw(convertFromRaw(blocks)) }); ``` and `originalState` could be declared in constructor as well?

jamesdools · 2019-08-12T15:04:56Z

packages/components/timed-text-editor/UpdateTimestamps/stt-align-node.js

+
+/**
+ *
+ * @param {array} sttData - array of STT words


change this comment to sttWords or change the param name?

It's more important to be clear whether it's an array of word objects (inc start / end times) or just strings

jamesdools · 2019-08-12T15:19:32Z

packages/components/timed-text-editor/UpdateTimestamps/stt-align-node.js

+  // transcriptData = [{} for _ in range(len(transcriptWords))]
+  const transcriptData = [];
+  // empty objects as place holder
+  transcriptWords.forEach(() => {


Unclear why we need to make an empty array of objects this way.

If we splice out the equal matches later (#L155), then the order (exact indices) doesn't matter, right?

Does it make sense to flip it - so we just collect the ones that don't match?

Yeah, I think it's coz originally this algo is mirroring the python code form @chrisbaume https://github.com/bbc/stt-align - https://github.com/bbc/stt-align-node

Ideally we could tweak it, refactor it there, and then bring it in via npm.
So leaving this for now, before they start to diverge too much.

jamesdools · 2019-08-12T15:57:24Z

packages/components/timed-text-editor/UpdateTimestamps/index.js

@@ -0,0 +1,82 @@
+import generateEntitiesRanges from '../../../stt-adapters/generate-entities-ranges/index.js';


can omit the .js extensions like elswhere 👍

jamesdools · 2019-08-12T15:58:19Z

packages/components/timed-text-editor/UpdateTimestamps/index.js

+const convertContentToText = (content) => {
+  var text = [];
+
+  for (var blockIdx in content.blocks) {


Reads weird - blockIndex or blockId? contextually 'index' is fine tbh.

going for blockIndex

jamesdools · 2019-08-12T15:59:25Z

packages/components/timed-text-editor/UpdateTimestamps/index.js

+    text = text.concat(blockArray);
+  }
+
+  return (text);


no parentheses needed

jamesdools · 2019-08-12T16:00:14Z

packages/components/timed-text-editor/UpdateTimestamps/index.js

+  return (text);
+};
+
+const createEntity = (start, end, confidence, word, wordIdx) => {


wordIdx -> wordIndex, or index

jamesdools · 2019-08-12T16:00:51Z

packages/components/timed-text-editor/UpdateTimestamps/index.js

+
+const createContentFromEntityList = (currentContent, newEntities) => {
+  // Update entites to block structure.
+  var updatedBlockArray = [];


not using var, as per Pietro's comment 👍

pietrop · 2019-08-12T17:40:51Z

Awesome! thanks @jamesdools I'll have a closer look and add those changes as a separate PR to #175 branch tomorrow

Fixed from James comments from #144 (review)

pietrop · 2019-08-13T17:09:03Z

addressed @jamesdools comments in PR #144 at commit #144 (review) so closing this PR

* added support for docx - needs better integration altho it works * adding timecodes and speakers to plain text export * develop: Fix 159 performance problem (#171) * moved header into a component with shouldComponentUopdate false to avoid unecessary re-renders - test * moved Header component into separate file +removed unused styling, commented out for now in case it's needed, eg in mobile view? * moved playback_rates const into separate file * optimised re-render of playback rate * optimise re-render for VideoPlayer * added some notes - draft on how to prevent uncessary re-renders in React * Added some comments * small note in docs about using console.log in render to measure performance * ammend to notes * Update 2019-05-16-prevent-unnecessary-re-renders-in-react.md * notes update * notes fix * trying out why-did-you-update * updated MediaPlayer and subcomponents * made ToolTip 'how does this work' into it's own component * updated Demo app to reduce unecessary re-renders * added react-visibility-sensor * refactor settings * removed unecessary attributes from state of components + WrapperBlock performance tweak using react-visibility-sensor * develop: Update timestamps diff (#172) * Added timestamp update via diff tool * Added missing function * Commited intermediate state * Rewrote timestamp alignment and differ to be integrated in each other instead of doing a 2-step process. * Update Timestamps now works correctly. * Fixed errors from rebase, removed debug code * Moved UpdateTimestamp into its own folder. * added updateTimestampsSSTAlign which updates the timestamps with the sst-align code * Added documentation * Merged timer for updating the timestamps and local save. * Selection state is now kept across updates to timestamps * Fixed bug where words with punctuation always are considered as new words. Timestamp update function now also uses the alignWords function directly instead of alignJSONText, removing some overhead. * Fixed small bug which raised an error if an empty block was present during timestamp update * Changed time of timestamp-update. Now re-calculates the timestamps after 5 seconds if the transcript has been edited or if the user saves the transcript manually with the save button * Code cleanup * develop: Murezzda update timestamps diff (#173) * Added timestamp update via diff tool * Added missing function * Commited intermediate state * Rewrote timestamp alignment and differ to be integrated in each other instead of doing a 2-step process. * Update Timestamps now works correctly. * Fixed errors from rebase, removed debug code * Moved UpdateTimestamp into its own folder. * added updateTimestampsSSTAlign which updates the timestamps with the sst-align code * Added timestamp update via diff tool * Added missing function * Commited intermediate state * Rewrote timestamp alignment and differ to be integrated in each other instead of doing a 2-step process. * Update Timestamps now works correctly. * Fixed errors from rebase, removed debug code * Moved UpdateTimestamp into its own folder. * added updateTimestampsSSTAlign which updates the timestamps with the sst-align code * Added documentation * Merged timer for updating the timestamps and local save. * Selection state is now kept across updates to timestamps * Fixed bug where words with punctuation always are considered as new words. Timestamp update function now also uses the alignWords function directly instead of alignJSONText, removing some overhead. * Fixed small bug which raised an error if an empty block was present during timestamp update * Changed time of timestamp-update. Now re-calculates the timestamps after 5 seconds if the transcript has been edited or if the user saves the transcript manually with the save button * Code cleanup * some changes to show sudgestions for PR * added some of changes sudgested in PR * Update package.json * develop: Murezzda update timestamps diff dpe groups words by speaker (#174) * Added timestamp update via diff tool * Added missing function * Commited intermediate state * Rewrote timestamp alignment and differ to be integrated in each other instead of doing a 2-step process. * Update Timestamps now works correctly. * Fixed errors from rebase, removed debug code * Moved UpdateTimestamp into its own folder. * added updateTimestampsSSTAlign which updates the timestamps with the sst-align code * Added timestamp update via diff tool * Added missing function * Commited intermediate state * Rewrote timestamp alignment and differ to be integrated in each other instead of doing a 2-step process. * Update Timestamps now works correctly. * Fixed errors from rebase, removed debug code * Moved UpdateTimestamp into its own folder. * added updateTimestampsSSTAlign which updates the timestamps with the sst-align code * Added documentation * Merged timer for updating the timestamps and local save. * Selection state is now kept across updates to timestamps * Fixed bug where words with punctuation always are considered as new words. Timestamp update function now also uses the alignWords function directly instead of alignJSONText, removing some overhead. * Fixed small bug which raised an error if an empty block was present during timestamp update * Changed time of timestamp-update. Now re-calculates the timestamps after 5 seconds if the transcript has been edited or if the user saves the transcript manually with the save button * Code cleanup * some changes to show sudgestions for PR * added some of changes sudgested in PR * adjusted DPE adapter so that it preserves paragraphs break within contiguos speakers * fixed one test * commented out auto align left aligning as a step before save btn and before export function, rather then as a step that happens everytime autosave is triggered, as that might be unecessary, and add performance overhead, I also noticed the cursor position jumped after realignement, thought something was been put in place to preserve/avoid that? * updated package-lock * fixed vulneranilities * Getting ready to publish alpha * 1.0.4-alpha.0 * 1.0.4@alpha * fixed notes * changing speaker and timecodes to be unselectable * 1.0.4-alpha.1 * 1.0.4-alpha.1 unselectabel speakers and timecodes * fix speaker and timecodes at paragraph level after realignement * 1.0.4-alpha.2 * fixed docx integration * Subtitles export (#168) * added option to export srt files and layout to export other type of captions with auto segmentation of lines * added support for other subtitles formats * fixed npm audit * implemented export in UI * fixed test added sample files for adding tests for subtitle composer module at later stage * added optional analytics for export download options * updated CSS * 1.0.4-alpha.3 * fixed subtiles segmentation algo was picking the wrong words from the list * moved PR template in github folder * cleaned up code for subtitles parsing * 1.0.4-alpha.4 * fixed alignement algo after interpolation words time boundares where overlapping * fixed interpolation * fixed filename of word doc export * fixed TimeBox and playback rate not working * 1.0.4-alpha.5 * removed scroll sync (#181) fix #180 * refactor changes from James review #160 * Develop branch - should component update refactor (#182) * Refactor should component updatre for transcript editor * Refactor should component updatre for PlayerControls * Refactor should component updatre for TimeBox * Refactor should component update for ProgressBar * Refactor should component update for TimedTextEditor * Refactor should component update for Header * fix from PR review Fixed from James comments from #144 (review) * Update package.json * Update package.json

murezzda force-pushed the update-timestamps-diff branch from fffe4ba to 80c9f34 Compare April 29, 2019 14:49

pietrop force-pushed the master branch 3 times, most recently from 6f07327 to 96f6e6d Compare May 29, 2019 16:27

murezzda added 12 commits June 3, 2019 10:12

Added timestamp update via diff tool

ecc6a51

Added missing function

8ce7124

Commited intermediate state

0872308

Rewrote timestamp alignment and differ to be integrated in each other…

2d1619b

… instead of doing a 2-step process.

Update Timestamps now works correctly.

5d92117

Fixed errors from rebase, removed debug code

02c82bc

Moved UpdateTimestamp into its own folder.

d0797d8

added updateTimestampsSSTAlign which updates the timestamps with the …

8babce7

…sst-align code

Added documentation

bf11847

Merged timer for updating the timestamps and local save.

09f7c16

Selection state is now kept across updates to timestamps

79dc2b9

Fixed bug where words with punctuation always are considered as new w…

91cccd1

…ords. Timestamp update function now also uses the alignWords function directly instead of alignJSONText, removing some overhead.

murezzda force-pushed the update-timestamps-diff branch from 80c9f34 to 91cccd1 Compare June 5, 2019 13:10

Fixed small bug which raised an error if an empty block was present d…

d4e5561

…uring timestamp update

murezzda added 2 commits June 10, 2019 18:55

Changed time of timestamp-update. Now re-calculates the timestamps af…

968e570

…ter 5 seconds if the transcript has been edited or if the user saves the transcript manually with the save button

Code cleanup

4d62770

pietrop mentioned this pull request Jun 23, 2019

suggestions for PR bbc#144 murezzda/react-transcript-editor#1

Draft

pietrop reviewed Jun 23, 2019

View reviewed changes

pietrop changed the title ~~Update timestamps via diff-match-patch~~ Update timestamps using alignment algorithm Jul 16, 2019

pietrop changed the title ~~Update timestamps using alignment algorithm~~ Update timestamps using diff alignment algorithm Jul 19, 2019

pietrop mentioned this pull request Jul 31, 2019

develop: Fix 159 performance problem #171

Merged

Pietro added 2 commits July 31, 2019 16:42

Merge branch 'develop' into update-timestamps-diff

e059936

pietrop mentioned this pull request Jul 31, 2019

Develop #175

Merged

pietrop mentioned this pull request Aug 12, 2019

A way of preserving or restoring time-codes while or after editing the text #30

Closed

jamesdools approved these changes Aug 12, 2019

View reviewed changes

pietrop pushed a commit that referenced this pull request Aug 13, 2019

fix from PR review

058a271

Fixed from James comments from #144 (review)

pietrop closed this Aug 13, 2019

		@@ -0,0 +1,82 @@
		import generateEntitiesRanges from '../../../stt-adapters/generate-entities-ranges/index.js';

Update timestamps using diff alignment algorithm #144

Update timestamps using diff alignment algorithm #144

Conversation

murezzda commented Apr 12, 2019

pietrop commented Apr 12, 2019

Laurian commented Apr 12, 2019

murezzda commented Apr 12, 2019

murezzda commented Apr 15, 2019

pietrop commented Apr 15, 2019 • edited Loading

murezzda commented Apr 29, 2019

pietrop commented Apr 29, 2019

murezzda commented Jun 5, 2019

murezzda commented Jun 5, 2019

pietrop commented Jun 5, 2019

murezzda commented Jun 6, 2019

pietrop commented Jun 6, 2019

murezzda commented Jun 7, 2019 • edited Loading

pietrop commented Jun 7, 2019

murezzda commented Jun 11, 2019

pietrop left a comment • edited Loading

Choose a reason for hiding this comment

Testing - Ted talk

Timings in last word in paragraphs

Difference between pasting and typing

Questions

Other thoughts

Possible separate PRs

Next

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

pietrop commented Jun 23, 2019 • edited Loading

pietrop commented Jul 13, 2019

pietrop commented Jul 16, 2019 • edited Loading

steps to reproduce

results

Caveats

jamesdools left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jamesdools Aug 12, 2019 • edited by pietrop Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

pietrop commented Aug 12, 2019

pietrop commented Aug 13, 2019

pietrop commented Apr 15, 2019 •

edited

Loading

murezzda commented Jun 7, 2019 •

edited

Loading

pietrop left a comment •

edited

Loading

pietrop commented Jun 23, 2019 •

edited

Loading

pietrop commented Jul 16, 2019 •

edited

Loading

jamesdools Aug 12, 2019 •

edited by pietrop

Loading