cta-wave / dpctf-deploy

WAVE Streaming Media Test Suite – Devices: This is the repository to install and deploy this test suite. Please read the README.md file for installation information.
https://www.cta.tech/Resources/Standards/WAVE-Project/WAVE-Streaming-Media-Test-Suite-Devices
Other
2 stars 5 forks source link

Improve quick start documents for observation framework #62

Closed yanj-github closed 7 months ago

yanj-github commented 7 months ago

Currently, it is stated to run OF: ./analyse-recording.sh <mp4-filepath>

As far as I am aware we can run OF by passing additional arguments to it. This will be useful for the user to run OF with additional arguments such as enable debug log set range of detection etc. Please add to the document accordingly.

We will propose a suitable wording later.

jpiesing commented 7 months ago

@FritzHeiden There seems to be a lot of confusion about running the OF "raw" on the host PC or in WSL or inside Docker and, when run in WSL, whether it's able to talk to the test runner that's running in Docker.

FritzHeiden commented 7 months ago

The motiviation behind runner the OF in the docker container is to reduce dependencies and to making the setup process easier. Is there any part in the current quick start that causes this confusion? Regarding the communication with the test runner, I will improve the quick start to be more specific on how this should work

yanj-github commented 7 months ago

@FritzHeiden from out end can you add the following please?

"Observation framework can be run with specific mode enabled by passing some optional arguments. ./analyse-recording.sh " Please also add reference to "Additional Options" section from OF readme file.

gitwjr commented 7 months ago

@FritzHeiden I suggest making it clear that the user needs to build the OF as the first step in Phase 3. It is not clear from the current wording this needs to be done (prior to build-dof.sh). "Following steps should be sufficient to get started with dockerized version, more details at https://github.com/cta-wave/device-observation-framework".

jpiesing commented 7 months ago

@FritzHeiden I suggest making it clear that the user needs to build the OF as the first step in Phase 3. It is not clear from the current wording this needs to be done (prior to build-dof.sh). "Following steps should be sufficient to get started with dockerized version, more details at https://github.com/cta-wave/device-observation-framework".

The OF README describes how to install the OF outside docker so referring people to that is very confusing.

FritzHeiden commented 7 months ago

The updated documentation for phase 3 still wasn't merged. The updated version is now in master, please check again.

"Observation framework can be run with specific mode enabled by passing some optional arguments. ./analyse-recording.sh " Please also add reference to "Additional Options" section from OF readme file.

I can add this wording to the quick start. Would you consider giving the "Additional Options" section an anchor so I can link it directly? At the moment its just bold text.

jpiesing commented 7 months ago

I'm following the new QUICK_START and get an error as shown.

$ ./build-dof.sh [+] Building 56.2s (10/13) docker:default => [internal] load .dockerignore 0.1s => => transferring context: 75B 0.0s => [internal] load build definition from Dockerfile.dof 0.1s => => transferring dockerfile: 810B 0.0s => [internal] load metadata for docker.io/library/python:3.8-bookworm 2.4s => [ 1/10] FROM docker.io/library/python:3.8-bookworm@sha256:9110b418dc2d04746f1ccc99c20fca1cb72e6f298e3bf0c20b6369f76e88967f 14.8s => => resolve docker.io/library/python:3.8-bookworm@sha256:9110b418dc2d04746f1ccc99c20fca1cb72e6f298e3bf0c20b6369f76e88967f 0.0s => => sha256:49b40be4436eff6fe463f6977159dc727df37cabe65ade75c75c1caa3cb0a234 64.14MB / 64.14MB 7.9s => => sha256:9110b418dc2d04746f1ccc99c20fca1cb72e6f298e3bf0c20b6369f76e88967f 1.86kB / 1.86kB 0.0s => => sha256:e88393a58f956e0c224a2489fa277944ef20603af906d5c1098d6bdcc1c7cf59 7.38kB / 7.38kB 0.0s => => sha256:7bb465c2914923b08ae03b7fc67b92a1ef9b09c4c1eb9d6711b22ee6bbb46a00 49.55MB / 49.55MB 1.9s => => sha256:2b9b41aaa3c52ab268b47da303015b94ced04a1eb02e58860e58b283404974f4 24.05MB / 24.05MB 1.4s => => sha256:625008535504ab68868ca06d1bdd868dee92a9878d5b55fc240af7ceb38b7183 2.01kB / 2.01kB 0.0s => => sha256:c558fac597f8ecbb7a66712e14912ce1d83b23a92ca8b6ff14eef209ab01aff2 211.12MB / 211.12MB 7.2s => => extracting sha256:7bb465c2914923b08ae03b7fc67b92a1ef9b09c4c1eb9d6711b22ee6bbb46a00 1.2s => => sha256:11402150a57e537c64dc69a28bba37f13acdedd50d8788894398a7b774786e7d 6.39MB / 6.39MB 2.2s => => sha256:4532b04ec8f74504e51d2e559a91696cf1d2abb690871b18983ccaf7f75800f9 17.28MB / 17.28MB 2.9s => => sha256:4df0efb76c10b517a5e00bb6e8d1215e7425e48c6897ba7e858da16de78daabf 245B / 245B 3.1s => => extracting sha256:2b9b41aaa3c52ab268b47da303015b94ced04a1eb02e58860e58b283404974f4 0.3s => => sha256:951fd003e72c32d376eaba82f18c2e31e52ade7a970c732bd189a59b68b6efee 2.85MB / 2.85MB 3.3s => => extracting sha256:49b40be4436eff6fe463f6977159dc727df37cabe65ade75c75c1caa3cb0a234 2.1s => => extracting sha256:c558fac597f8ecbb7a66712e14912ce1d83b23a92ca8b6ff14eef209ab01aff2 3.7s => => extracting sha256:11402150a57e537c64dc69a28bba37f13acdedd50d8788894398a7b774786e7d 0.2s => => extracting sha256:4532b04ec8f74504e51d2e559a91696cf1d2abb690871b18983ccaf7f75800f9 0.3s => => extracting sha256:4df0efb76c10b517a5e00bb6e8d1215e7425e48c6897ba7e858da16de78daabf 0.0s => => extracting sha256:951fd003e72c32d376eaba82f18c2e31e52ade7a970c732bd189a59b68b6efee 0.2s => [ 2/10] RUN apt-get update && apt-get install -y git libzbar0 netcat-openbsd libgl1 ffmpeg python3-pyaudio portaudio19-dev 17.2s => [ 3/10] WORKDIR /usr/app 0.0s => [ 4/10] RUN git clone https://github.com/cta-wave/device-observation-framework.git 1.3s => [ 5/10] WORKDIR /usr/app/device-observation-framework 0.0s => [ 6/10] RUN ./install.sh 19.9s => ERROR [ 7/10] RUN cd audio_mezzanine && ./import_audio_mezzanine.sh && cd .. 0.3s


[ 7/10] RUN cd audio_mezzanine && ./import_audio_mezzanine.sh && cd ..:
0.345 /bin/sh: 1: ./import_audio_mezzanine.sh: Permission denied


Dockerfile.dof:11

9 |
10 | RUN ./install.sh 11 | >>> RUN cd audio_mezzanine && ./import_audio_mezzanine.sh && cd .. 12 |
13 | RUN mkdir /usr/app/recordings

ERROR: failed to solve: process "/bin/sh -c cd audio_mezzanine && ./import_audio_mezzanine.sh && cd .." did not complete successfully: exit code: 126

This machine doesn't have the test runner installed on it but that won't be unusual as people will want to run the OF on faster machines or pass a failing test off to a more qualified engineer to analyse.

If this is because the test runner isn't installed and there's no way to avoid that then a more friendly error message should be added.

yanj-github commented 7 months ago

@jpiesing the error you have raised above is now resolved.

yanj-github commented 7 months ago

@FritzHeiden

I can add this wording to the quick start. Would you consider giving the "Additional Options" section an anchor so I can link it directly? At the moment its just bold text.

"Additional Options" section should be linkable directly now.

yanj-github commented 7 months ago

@gitwjr and @jpiesing I have added new issue https://github.com/cta-wave/dpctf-deploy/issues/63 for connectivity issue from OF to test runner under WSL, as this is separate issue.

jpiesing commented 7 months ago

@jpiesing the error you have raised above is now resolved.

Confirmed.

jpiesing commented 7 months ago

I've reviewed this issue and all the comments. As far as I can see there's only one trivial change outstanding.

The link to the description of the options from the deploy README.md (renamed from QUICK_START.md) takes you to the OF repo. It doesn't take you to the text in the OF README.md that describes the options.

@FritzHeiden In the deploy README.md, please change the link "the documentation"

image

to be https://github.com/cta-wave/device-observation-framework/blob/main/README.md#additional-options

Then I think this issue can be closed.

FritzHeiden commented 7 months ago

done https://github.com/cta-wave/dpctf-deploy/pull/72

jpiesing commented 7 months ago

@yanj-github You created this issue. Please can you check if it can now be closed.

yanj-github commented 7 months ago

@jpiesing and @FritzHeiden I just noticed the deploy readme contains a section to record the capture.

position video recording device (e.g. smartphone with 120fps using AVC codec) in front of the display of DUT Note: Significant care is needed. Please see https://dash-large-files.akamaized.net/WAVE/assets/How-to-take-clear-recordings-v3.pptx .\

Can we link to "Obtain recording files" instead of just ppt file please? Obtain recording files contains more useful information for user must follow. And this is better in the future if we add another document for audio capture don't have to change the deploy readme again.

No further comments. Thanks

FritzHeiden commented 7 months ago

Done https://github.com/cta-wave/dpctf-deploy/pull/74. Please check

yanj-github commented 7 months ago

Thank you very much @FritzHeiden Looks all good and closing the issue.